Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

main branch not compat with remark-highlightjs? #36

Closed
NullVoxPopuli opened this issue May 1, 2021 · 16 comments
Closed

main branch not compat with remark-highlightjs? #36

NullVoxPopuli opened this issue May 1, 2021 · 16 comments

Comments

@NullVoxPopuli
Copy link

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: 869119b)
remark-highlightjs: 6.0.0

@NullVoxPopuli
Copy link
Author

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
Copy link
Author

NullVoxPopuli commented May 1, 2021

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

oof

@NullVoxPopuli
Copy link
Author

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

@wooorm
Copy link
Owner

wooorm commented May 1, 2021

We’re switching to ESM: unifiedjs/unified#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
Copy link
Author

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

@wooorm
Copy link
Owner

wooorm commented May 1, 2021

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
Copy link
Author

👍

I made this change
❯ 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
Copy link
Author

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
Copy link
Author

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

@NullVoxPopuli
Copy link
Author

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
Copy link
Owner

wooorm commented May 1, 2021

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
Copy link
Author

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
Copy link
Owner

wooorm commented May 1, 2021

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
Copy link
Author

NullVoxPopuli commented May 1, 2021

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
Copy link
Owner

wooorm commented May 1, 2021

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!

@wooorm wooorm closed this as completed May 1, 2021
@NullVoxPopuli
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants