wooorm / lowlight

Virtual syntax highlighting for virtual DOMs and non-HTML things
MIT License
746 stars 26 forks source link

`main` branch not compat with remark-highlightjs? #36

Closed NullVoxPopuli closed 3 years ago

NullVoxPopuli commented 3 years ago

Currently getting: image

TypeError: Cannot read property 'highlight' of undefined

which occurs on this code:

data.hChildren = _lowlight.default.highlight(lang, node.value, {
  prefix
}).value;

which is transform via babel-jest -> babel -> @babel/plugin-transform-modules-commonjs

Versions: lowlight main (as of today: 869119be9a7f12eb5da0092800a7b4622d70499b) remark-highlightjs: 6.0.0

NullVoxPopuli commented 3 years ago

if my babel config for jest is now this:

module.exports = {
  env: {
    test: {
      plugins: [
        [
          require('@babel/plugin-transform-modules-commonjs'),
          {
            importInterop: 'node',
          },
        ],
      ]}}};

then I get this error:

 FAIL  tests/integration/remark.test.cjs
  ● Test suite failed to run

    TypeError: languageDefinition.bind is not a function

       9 |
      10 | export function registerLanguage(hljs) {
    > 11 |   hljs.registerLanguage('glimmer', _glimmer);
         |        ^
      12 | }
      13 |
      14 | export function registerInjections(hljs) {

      at Object.registerLanguage (../node_modules/highlight.js/lib/core.js:2134:45)
      at registerLanguage (../src/index.js:11:8)
      at Object.<anonymous> (-utils.js:7:1)
      at Object.<anonymous> (integration/remark.test.cjs:10:17)
NullVoxPopuli commented 3 years ago

ok, so with importInterop: 'node' all my modules break and no longer work 🤔

oof

NullVoxPopuli commented 3 years ago

so I guess... how does lowlight / remark / etc test against cjs?

wooorm commented 3 years ago

We’re switching to ESM: https://github.com/unifiedjs/unified/issues/121. This post, linked to from the readme, goes into a bit more detail: https://gist.github.com/sindresorhus/a39789f98801d908bbc7ff3ecc99d99c. Specifically, lowlight#main does not have a default export

lowlight on main is waiting for the next major of highlight.js before being released, though.

NullVoxPopuli commented 3 years ago

so it wouldn't be possible to have an alpha release? :D

wooorm commented 3 years ago

You should be able to use wooorm/lowlight in your package.json to install the code from GH? 🤔 It doesn’t include types though.

Could you try that? If it doesn’t work, I can make a beta.

NullVoxPopuli commented 3 years ago

:+1:

I made this change ```diff ❯ gd diff --git a/package.json b/package.json index 69a7a33..f8fb698 100644 --- a/package.json +++ b/package.json @@ -39,7 +39,7 @@ "eslint": "^7.23.0", "eslint-plugin-html": "^6.1.2", "highlight.js": ">= 11.0.0-alpha0", - "lowlight": "github:NullVoxPopuli/lowlight#add-get-language", + "lowlight": "github:wooorm/lowlight", "jest": "^26.6.3", "rehype": "^11.0.0", "rehype-highlight": "^4.1.0", diff --git a/tests/integration/rehype.test.cjs b/tests/integration/rehype.test.cjs index 13630d5..c80ad1e 100644 --- a/tests/integration/rehype.test.cjs +++ b/tests/integration/rehype.test.cjs @@ -6,9 +6,9 @@ const highlight = require('rehype-highlight'); const hljs = require('highlight.js'); const { setup } = require('../../dist/glimmer.cjs.cjs'); -const { tag } = require('../-utils'); +// const { tag } = require('../-utils'); -setup(hljs); +// setup(hljs); function parse(text) { return rehype() @@ -18,7 +18,7 @@ function parse(text) { // same error: lowlight.registerLanguage is not a function // // glimmer: require('highlight.js/lib/languages/handlebars'), - // hbs: require('highlight.js/lib/languages/handlebars'), + hbs: require('highlight.js/lib/languages/handlebars'), // glimmer: setup, // glimmer: hljs.getLanguage('glimmer'), }, diff --git a/yarn.lock b/yarn.lock index 1654a31..e1c8caa 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5764,9 +5764,9 @@ lower-case@^2.0.2: dependencies: tslib "^2.0.3" -lowlight@^1.10.0, lowlight@^1.2.0, "lowlight@github:NullVoxPopuli/lowlight#add-get-language": +lowlight@^1.10.0, lowlight@^1.2.0, "lowlight@github:wooorm/lowlight": version "1.20.0" - resolved "https://codeload.github.com/NullVoxPopuli/lowlight/tar.gz/f93d85903e278129a2579e977fec683ac3aa3d36" + resolved "https://codeload.github.com/wooorm/lowlight/tar.gz/869119be9a7f12eb5da0092800a7b4622d70499b" dependencies: "@types/hast" "^2.0.0" fault "^2.0.0" ```

I made sure to use an existing language to make sure it wasn't an issue with my plugin, but I still get:

FAIL  tests/integration/rehype.test.cjs
  ● Rehype › works

    TypeError: lowlight.registerLanguage is not a function

      24 |       },
      25 |     })
    > 26 |     .processSync(text)
         |      ^
      27 |     .toString();
      28 | }
      29 |

      at Function.<anonymous> (../node_modules/rehype-highlight/core.js:21:18)
      at freeze (../node_modules/unified/index.js:121:31)
      at Function.processSync (../node_modules/unified/index.js:377:5)
      at parse (integration/rehype.test.cjs:26:6)
      at Object.<anonymous> (integration/rehype.test.cjs:33:7)]

but, that's because lowlight no longer has a default export. So if I edit my node_modules/rehype-highlight/core.js to have lowlight = lowlight.lowlight;, then I get passed that error.

But, no matter what I do, I can't get my code block higlighted. at most, the hljs class is added to the code element :-\

NullVoxPopuli commented 3 years ago

also, if I specify my own grammar, I get this error:

   TypeError: Cannot read property 'name' of undefined

      31 |     })
      32 |     .use(html)
    > 33 |     .processSync(text)
         |      ^
      34 |     .toString();
      35 | }
      36 |

      at Object.registerLanguage (../node_modules/lowlight/node_modules/highlight.js/lib/core.js:2312:15)
      at Object.registerLanguage (../node_modules/lowlight/lib/core.js:137:8)
      at Function.<anonymous> (../node_modules/rehype-highlight/core.js:23:18)
      at freeze (../node_modules/unified/index.js:121:31)

which tells me that there might be an incompatibility with what rehype-highlight is expecting and how my grammar works when used in cjs directly with hljs

NullVoxPopuli commented 3 years ago

ok, for this name error it looks like my setup function must return the base grammar

NullVoxPopuli commented 3 years ago

ok, so that I've fixed issues with my own grammar, the issue where nothing gets highlighted is still present.

This is what I'm doing:

const { stripIndent } = require('common-tags');
const rehype = require('rehype');
const markdown = require('remark-parse');
const remark2rehype = require('remark-rehype');
const highlight = require('rehype-highlight');
const html = require('rehype-stringify')
const hljs = require('highlight.js');

const { externalSetup } = require('../../dist/glimmer.cjs.cjs');
const { tag } = require('../-utils');

function parse(text) {
  debugger;
  return rehype()
    .data('settings', { fragment: true })
    .use(markdown)
    .use(remark2rehype)
    .use(highlight, {
      languages: {
        // none of these highlight anything
        // glimmer: require('highlight.js/lib/languages/handlebars'),
        // hbs: require('highlight.js/lib/languages/handlebars'),
        // js: require('highlight.js/lib/languages/javascript'),
        // glimmer: hljs.getLanguage('glimmer'),
       //  glimmer: externalSetup,
      },
    })
    .use(html)
    .processSync(text)
    .toString();
}

describe('Rehype', () => {
  it('works', async () => {
    expect(
      // I am remembering to change the language tag here ;)
      parse(stripIndent`
      \`\`\`hbs
        {{@arg}}
      \`\`\`
      `)
    ).toEqual(
      '<pre><code class="hljs language-glimmer">' +
        tag('punctuation mustache', ['{{', tag('punctuation', '@'), tag('params', 'arg'), '}}']) +
        '</code></pre>'
    );
  });
});
wooorm commented 3 years ago

Did you ever actually get this to work with highlight.js itself? I think your grammar is incorrect.

So, I’m in the codebase of rehype-highlight, with an example.html:

<h1>Hello World!</h1>

<pre><code class="language-xxx">whatever</code></pre>

And example.js:

var vfile = require('to-vfile')
var report = require('vfile-reporter')
var rehype = require('rehype')
var highlight = require('./index.js')
var glimmer = require('highlightjs-glimmer').glimmer

console.log('xxx:', glimmer)

rehype()
  .data('settings', {fragment: true})
  .use(highlight, {
    languages: {
      xxx: glimmer
    }
  })
  .process(vfile.readSync('example.html'), function (error, file) {
    console.error(report(error || file))
    console.log(String(file))
  })

When I run that, I get an error:

TypeError: Cannot read property 'length' of undefined
    at ~/rehype-highlight/node_modules/lowlight/node_modules/highlight.js/lib/core.js:420:15
    at Array.map (<anonymous>)
    at join (~/rehype-highlight/node_modules/lowlight/node_modules/highlight.js/lib/core.js:414:18)
    at MultiRegex.compile (~/rehype-highlight/node_modules/lowlight/node_modules/highlight.js/lib/core.js:880:31)
    at ResumableMultiRegex.getMatcher (~/rehype-highlight/node_modules/lowlight/node_modules/highlight.js/lib/core.js:951:15)
    at ResumableMultiRegex.exec (~/rehype-highlight/node_modules/lowlight/node_modules/highlight.js/lib/core.js:972:22)
    at _highlight (~/rehype-highlight/node_modules/lowlight/node_modules/highlight.js/lib/core.js:2003:35)
    at Object.highlight (~/rehype-highlight/node_modules/lowlight/node_modules/highlight.js/lib/core.js:1626:9)
    at Object.highlight (~/rehype-highlight/node_modules/lowlight/lib/core.js:48:17)
    at visitor (~/rehype-highlight/core.js:67:22)

Going into node_modules/lowlight/node_modules/highlight.js/lib/core.js, and adding a console.log there (L419):

console.log('yyy:', regex, re);

I get:

yyy: /\{\{!--/ \{\{!--
yyy: /\{\{!/ \{\{!
yyy: /<!--/ <!--
yyy: /&[a-z]+;|&#[0-9]+;|&#x[a-f0-9]+;/ &[a-z]+;|&#[0-9]+;|&#x[a-f0-9]+;
yyy: \{\{\{?#? \{\{\{?#?
yyy: /\=/ \=
yyy: [ /@/, /[\w\d-_]+/ ] undefined

It looks like hljs can’t handle your regex groups?

https://github.com/NullVoxPopuli/highlightjs-glimmer/blob/dd73a157ef7548353cf112cd246ee813355a71d2/src/glimmer.js#L115

NullVoxPopuli commented 3 years ago

Did you ever actually get this to work with highlight.js itself? I think your grammar is incorrect.

yes. it is not incorrect. here is a demo: https://hljs-glimmer.nullvoxpopuli.com I'm using hljs v11 tho These demos use ES Modules, as requirejs doesn't exist in the browser. I also have a Node test here: https://github.com/NullVoxPopuli/highlightjs-glimmer/blob/main/tests/node.test.cjs

wooorm commented 3 years ago

OK, but if it only works with an unreleased version of highlight.js, then it’s not too weird that it doesn’t work with lowlight, or rehype-highlight, either?

Can you make it work with hljs 10?

NullVoxPopuli commented 3 years ago

then it’s not too weird that it doesn’t work

Yeah, I've mostly been surprised that I can't force the highlightjs dependency. There are enough differences between main and release in all the remark/rehype/lowlight dependencies that I can't just swap stuff out in node_modules

Can you make it work with hljs 10?

I cannot -- hljs v11 has a ton of new grammar authoring features which are really nice :D

wooorm commented 3 years ago

You could copy/paste or fork rehype-highlight for now. It’ll take a month or so for me to get there. And use lowlight#main for now (I don’t expect much to change for the next release).

But for this issue: yes, the main branch is not compatible with rehype-highlight, mostly due to ESM. I’ll get to it (in that project) when I can!

NullVoxPopuli commented 3 years ago

all good! at least the issue is known! :D