micromark-util-character: fix crash on some Node.js versions

[aduh95]

Initial checklist

• I read the support docs
• I read the contributing guide
• I agree to follow the code of conduct
• I searched issues and couldn’t find anything (or linked relevant results below)
• If applicable, I’ve added docs and tests

Description of changes

\p regex notation is not available on Node.js build without intl support, causing the process to crash even the regex is never used. Ideally we would have a regex that works everywhere, but I figure lazy-loading the same regex should be enough for most use-cases.

Refs: nodejs/node#49988 (comment)

[wooorm]

Lazy loading isn’t going to solve it. This regex is required to parse markdown.

Hmmm. I literally started using this because this is now available in modern browsers and Node 16+ (https://github.com/micromark/micromark/releases/tag/4.0.0).

I feel like we should be able to depend on recent Unicode?

[aduh95]

Lazy loading isn’t going to solve it. This regex is required to parse markdown.

Bummer, also I remember now that a SyntaxError would still be emitted no matter if the code is executed. Maybe we could try using the RegExp constructor with a string literal (i.e. new RegExp('\\p{P}', 'u')); node is still compiling on my machine btw, I haven't tested any of it yet, I'm just throwing ideas.

I feel like we should be able to depend on recent Unicode?

Node.js supports being build without any unicode support, I've suggested deprecating it that in nodejs/node#35942, you can read through the responses to see what are the use cases for this.
In the nodejs/node repo, we need to be able to run the test suite when node is compiled in this mode, which blocks us from updating our doc dependencies.

[wooorm]

Like, markdown uses unicode. We could inject all those unicode characters in the regex again (like before), but that feels like going against someone specifically compiling without unicode support?

Node.js supports being build without any unicode support

This might be out of Nodes control but couldn’t it then use ascii punctuation in this case?

[wooorm]

So your problem is that the Node project doesn’t compile. If it were to compile, it doesn’t actually parse markdown, so it wouldn’t get an error?

[aduh95]

The problem is that we need to build the docs before running the tests (because some tests ensure some specific parts of the documentation matches the actual implementation), and the way the CI is configured today, it's using the binary freshly built, which doesn't have unicode support.

new RegExp('\\p{P}', 'u'))

I've tried that, and as you predicted, it didn't help because the code is still run.
const unicodePunctuationInternal = () => false seems to work, but obviously that's not a great fix 😅

This might be out of Nodes control but couldn’t it then use ascii punctuation in this case?

I'm not sure what you mean by that, I think Node.js documentation is ASCII-only.

[wooorm]

That for \P, when you build as ascii, to use ascii punctuation, instead of unicode punctuation

[wooorm]

Closing as abandoned. I still kinda feel like, if v8 is built to not include unicode knowledge, then \p{P} in a regex should perhaps not crash, but instead limit itself to the ASCII range?
The alternative, of tools that has knowledge of unicode to prebuilt unicode indexes, I dunno that feels weird.

If there was some condition that Node would pick up on when built without unicode capabilities, we could perhaps do something about providing two versions for files here.

[aduh95]

If there was some condition that Node would pick up on when built without unicode capabilities, we could perhaps do something about providing two versions for files here.

I would have to doublecheck, but I think process.versions.unicode would be undefined when built without Unicode support. EDIT: yes this is correct, process.versions.unicode and process.versions.icu are defined only node was compiled with support for i18n.

[wooorm]

Hmm, maybe interesting, but I was hoping for conditions instead of Node-specific process

[aduh95]

conditions could work, if the package was exposing a "without-intl" exports, we could spawn our process with node -C without-intl. Should I make a PR like that?

[wooorm]

And what would it do?