Skip to content

doc build problem in withoutintl builds #41077

New issue
New issue
Closed
#41091
Closed
doc build problem in withoutintl builds#41077
#41091
Labels
docIssues and PRs related to Node.js documentation.

Description

@richardlau
richardlau
opened on Dec 3, 2021

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

Activity

  1. richardlau commented on Dec 3, 2021

    @richardlau
    richardlau
    on Dec 3, 2021
    MemberAuthor

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

  2. richardlau commented on Dec 3, 2021

    @richardlau
    richardlau
    on Dec 3, 2021
    MemberAuthor

  3. targos commented on Dec 4, 2021

    @targos
    targos
    on Dec 4, 2021
    Member

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

  4. Mesteery
    added
    docIssues and PRs related to Node.js documentation.
    on Dec 4, 2021
  5. nodejs-github-bot
    added a commit that references this issue on Dec 4, 2021

    tools: rollback highlight.js

    c802c4b

  6. Trott commented on Dec 4, 2021

    @Trott
    Trott
    on Dec 4, 2021
    Member

    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

  7. Trott commented on Dec 4, 2021

    @Trott
    Trott
    on Dec 4, 2021
    Member

    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?

  8. richardlau commented on Dec 4, 2021

    @richardlau
    richardlau
    on Dec 4, 2021
    MemberAuthor

    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?

  9. Trott commented on Dec 4, 2021

    @Trott
    Trott
    on Dec 4, 2021
    Member

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

  10. Trott commented on Dec 4, 2021

    @Trott
    Trott
    on Dec 4, 2021
    Member

    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.

  11. Trott commented on Dec 4, 2021

    @Trott
    Trott
    on Dec 4, 2021
    Member

    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.

  12. Trott commented on Dec 4, 2021

    @Trott
    Trott
    on Dec 4, 2021
    Member

    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.

  13. Trott commented on Dec 4, 2021

    @Trott
    Trott
    on Dec 4, 2021
    Member

    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?)

  14. aduh95 commented on Dec 4, 2021

    @aduh95
    aduh95
    on Dec 4, 2021
    Contributor

    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

  15. 13 remaining items

  16. danielleadams
    added 3 commits that reference this issue on Dec 14, 2021

    tools: rollback highlight.js

    9cfdf15

    build: skip documentation generation if no ICU

    831face

    tools: update highlight.js to 11.3.1

    7b606cf
  17. danielleadams
    added 6 commits that reference this issue on Jan 31, 2022

    tools: rollback highlight.js

    51253a3

    build: skip documentation generation if no ICU

    0e692e2

    tools: update highlight.js to 11.3.1

    1721048

    tools: rollback highlight.js

    e0cc79d

    build: skip documentation generation if no ICU

    519d64c

    tools: update highlight.js to 11.3.1

    71d7928
  18. danielleadams
    added 3 commits that reference this issue on Feb 1, 2022

    tools: rollback highlight.js

    f9cf50f

    build: skip documentation generation if no ICU

    860d552

    tools: update highlight.js to 11.3.1

    1ff1521
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    docIssues and PRs related to Node.js documentation.

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions