doc build problem in withoutintl builds

[richardlau]

Version

v18.0.0-pre

Platform

No response

Subsystem

build

What steps will reproduce the bug?

Run CI. https://ci.nodejs.org/job/node-test-commit-linux-containered/nodes=ubuntu1804_sharedlibs_withoutintl_x64/ builds have been consistently failing starting today. These are run with python3 ./configure --verbose --without-intl.

e.g. From today's daily build:
https://ci.nodejs.org/job/node-test-commit-linux-containered/29799/nodes=ubuntu1804_sharedlibs_withoutintl_x64/console

How often does it reproduce? Is there a required condition?

Seems consistent for builds today.

What is the expected behavior?

Build completes.

What do you see instead?

08:05:45 /home/iojs/build/workspace/node-test-commit-linux-containered/tools/doc/node_modules/highlight.js/lib/languages/python.js:10
08:05:45   const IDENT_RE = /[\p{XID_Start}_]\p{XID_Continue}*/u;
08:05:45                    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
08:05:45 
08:05:45 SyntaxError: Invalid regular expression: /[\p{XID_Start}_]\p{XID_Continue}*/: Invalid property name in character class
08:05:45     at Object.compileFunction (node:vm:352:18)
08:05:45     at wrapSafe (node:internal/modules/cjs/loader:1026:15)
08:05:45     at Module._compile (node:internal/modules/cjs/loader:1061:27)
08:05:45     at Object.Module._extensions..js (node:internal/modules/cjs/loader:1149:10)
08:05:45     at Module.load (node:internal/modules/cjs/loader:975:32)
08:05:45     at Function.Module._load (node:internal/modules/cjs/loader:822:12)
08:05:45     at Module.require (node:internal/modules/cjs/loader:999:19)
08:05:45     at require (node:internal/modules/cjs/helpers:102:18)
08:05:45     at Object.<anonymous> (/home/iojs/build/workspace/node-test-commit-linux-containered/tools/doc/node_modules/highlight.js/lib/index.js:144:33)
08:05:45     at Module._compile (node:internal/modules/cjs/loader:1097:14)
08:05:45 

Additional information

No response

[richardlau]

The error was thrown from highlight.js, which was updated in #41036 (and deemed to not require a full CI 😞).

[richardlau]

• CI build with 42e13c2 (tool/doc packages update):
  https://ci.nodejs.org/job/node-test-commit-linux-containered/29823/nodes=ubuntu1804_sharedlibs_withoutintl_x64/ (❌)
• CI build with c8a0404 (preceding commit):
  https://ci.nodejs.org/job/node-test-commit-linux-containered/29822/nodes=ubuntu1804_sharedlibs_withoutintl_x64/ (✔️)

[targos]

I suppose that this regular expression requires ICU data to work.

[Trott]

Does the regexp literal stop throwing an error in no-icu builds if we change XID to ID?

/[\p{ID_Start}_]\p{ID_Continue}*/u;

EDIT: Based on the table in https://nodejs.org/dist/latest-v17.x/docs/api/intl.html#options-for-building-nodejs, I'd expect that it still wouldn't work, but I'm building a non-icu node right now to confirm.

EDIT: I also don't see any elegant way to do feature detection for regexp property class names other than try / catch. Correction welcome.

@nodejs/i18n-api

[Trott]

EDIT: Based on the table in https://nodejs.org/dist/latest-v17.x/docs/api/intl.html#options-for-building-nodejs, I'd expect that it still wouldn't work, but I'm building a non-icu node right now to confirm.

On the other hand, the spec seems to say that ID_START is required but XID_START is not, so maybe V8 handles it even without Node.js compiling ICU?

[richardlau]

Fwiw the highlight.js change that introduced the regexp is highlightjs/highlight.js#3280. Do we highlight Python anywhere in our docs? Maybe we could minimally load highlight.js and only register the languages we expect to highlight in our docs?

[Trott]

If all else fails, another possibility might be to skip building (or at least syntax highlighting) docs on without-intl builds.

[Trott]

Does the regexp literal stop throwing an error in no-icu builds if we change XID to ID?

Confirmed that it throws an error still with that change.

[Trott]

Do we highlight Python anywhere in our docs?

Based on grep -rl '```py' doc/, it seems that we do not.

Maybe we could minimally load highlight.js and only register the languages we expect to highlight in our docs?

That seems like a feature highlight.js supports, based on https://www.npmjs.com/package/highlight.js.

[Trott]

That seems like a feature highlight.js supports, based on https://www.npmjs.com/package/highlight.js.

LOL! This is the way we're actually doing it already, but we're still importing everything at the outset. The fix here is a single line change to html.mjs. PR coming in a few minutes.

[Trott]

Also, I'm kind of wondering if we want to require at least a minimal intl. I'm not sure what the use case is for --without-intl though so maybe that's short-sighted. (Maybe it's for a small build size for constrained environments like IoT?)

[aduh95]

Also, I'm kind of wondering if we want to require at least a minimal intl. I'm not sure what the use case is for --without-intl though so maybe that's short-sighted. (Maybe it's for a small build size for constrained environments like IoT?)

Related: #35942

[Trott]

Oof... make lint-js fails on --without-intl builds for similar reasons.

[aduh95]

Oof... make lint-js fails on --without-intl builds for similar reasons.

It's been a while I think.. I noticed it when working on deprecating punycode, but since no one complained I assumed it was a known limitation. Commenting out the faulty regex locally works btw, maybe replacing it with a new RegExp call would too.