doc: add doc for --loader option by SirR4T · Pull Request #22104 · nodejs/node · GitHub

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
documentation is changed or added
commit message follows commit guidelines

s/customer/custom

Also, it should be noted that this is still an experimental feature.

I believe it's been brought up before but given that --loader is still experimental I think it would it be better if it were --experimental-loader in much the same way other experimental stuff is prefixed with --experimental-

@jdalton given you have to run node with --experimental-modules anyway to use this - why?

thanks @jasnell ! fixed the typo.
Should i mention here, that it needs to be used with --experimental-modules?

@benjamingr

given you have to run node with --experimental-modules anyway to use this - why?

I like the consistency of the --experimental- prefix for experimental things. The prefix makes it explicit that the feature/sub-feature is experimental.

esm.html#esm_loader_hooks?

thanks! fixed now.

cc @nodejs/modules-active-members?

It seems doc/node.1 and PrintHelp() in src/node.cc should also be updated?

@vsemozhetbyt Added docs for node.1 and src/node.cc, though I couldn't maintain consistency in the doc strings. Is that fine?

cc @nodejs/documentation for node.1 and src/node.cc changes.

Linter issue:

doc/api/cli.md
  1:1  warning  Missing newline character at end of file  final-newline  remark-lint

@vsemozhetbyt thanks, installed markdown linter now. Will ensure i run this, before pushing any more doc updates.

cc @nodejs/build-infra re CI error:

not ok 2295 sequential/test-fs-watch
  ---
  duration_ms: 0.111
  severity: fail
  exitcode: 1
  stack: |-
    internal/fs/watchers.js:170
        throw error;
        ^
    
    Error: ENOSPC: no space left on device, watch '/home/travis/build/nodejs/node/test/.tmp.0/watch.txt'

@nodejs/documentation Can anybody confident enough look into node.1 and node.cc changes so we could have some more LGTM?

Another CI: https://ci.nodejs.org/job/node-test-pull-request/16313/

If CI is green, I will land this PR tomorrow if nobody objects.

LGTM! 🎉

In last CIs, it seems we have some repetitive fails in parallel/test-cli-node-print-help in node-test-commit-linux-containered that may be relative. Can anybody look into?

One more CI to be sure: https://ci.nodejs.org/job/node-test-pull-request/16332/

@vsemozhetbyt sorry, this maybe completely off topic, but why are --experimental-modules, --experimental-vm-modules, and --preserve-symlinks predicated on having i18n support?

Sorry, this also puzzled me and I thought that it was because I just did not know C++ :)

@jdalton : updated the doc for --loader option. Sounds good? Open to other wordings / text as well, to put the "experimental" point across.

@SirR4T It's better. But --loader is also experimental. So maybe something like

Specify the file of the custom [experimental ECMAScript module][] loader.

thanks, @jdalton , fixed.

Nit: reference list is sorted in ASCII order, so this item needs to be placed before the [libuv threadpool documentation].

Sure, will fix that. Any way we could add this as a rule to make lint?

It would be helpful to have it as a rule as we are already a bit messy in some docs in these sections,

Would updating remark-preset-lint-node to also include remark-lint-alphabetize-lists work? I expect this would cause build failures, if remark-preset-lint-node was updated before fixes for that landed in master. How would the transition be handled?

I am not sure that remark-lint-alphabetize-lists can fix this case as reference lists are not common markdown lists. Maybe @rubys can tell if there is a ready easy solution for this?

Remark calls those lines definitions. Looking at the source to remark-lint-alphabetize-lists
, if list where changed to definition on line 18, the code would do what you want.

#22271 is merged, so let's unblock and run CI:
https://ci.nodejs.org/job/node-test-pull-request/16490/

Rebuild failed job: https://ci.nodejs.org/job/node-test-commit-linux-containered/6326/
And again: https://ci.nodejs.org/job/node-test-commit-linux-containered/6335/

Landed in 9d6619e
Thank you!

PR-URL: #22104 Fixes: #21230 Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com> Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: John-David Dalton <john.david.dalton@gmail.com>

nodejs-github-bot added cli Issues and PRs related to the Node.js command line interface. doc Issues and PRs related to the documentations. labels Aug 3, 2018

jasnell reviewed Aug 3, 2018

View reviewed changes

vsemozhetbyt reviewed Aug 3, 2018

View reviewed changes

vsemozhetbyt added the esm Issues and PRs related to the ECMAScript Modules implementation. label Aug 3, 2018

SirR4T force-pushed the addCliDocForLoader branch from 2406e35 to 9d6356d Compare August 6, 2018 05:08

This comment has been minimized.

Sign in to view

benjamingr approved these changes Aug 7, 2018

View reviewed changes

SirR4T force-pushed the addCliDocForLoader branch from 0dea374 to 341d4b0 Compare August 8, 2018 06:03

This comment has been minimized.

Sign in to view

jasnell approved these changes Aug 8, 2018

View reviewed changes

vsemozhetbyt mentioned this pull request Aug 8, 2018

test: remove common.fileExists() #22151

Closed

3 tasks

This comment has been minimized.

Sign in to view

SirR4T force-pushed the addCliDocForLoader branch from 341d4b0 to a88503c Compare August 9, 2018 04:19

This comment has been minimized.

Sign in to view

SirR4T force-pushed the addCliDocForLoader branch from a88503c to b4d15f5 Compare August 10, 2018 05:14

sagirk approved these changes Aug 10, 2018

View reviewed changes

SirR4T force-pushed the addCliDocForLoader branch from b4d15f5 to b4897b6 Compare August 16, 2018 04:45

doc: mention that --loader option is experimental

188fb32

SirR4T force-pushed the addCliDocForLoader branch from b4897b6 to 188fb32 Compare August 16, 2018 06:26

vsemozhetbyt reviewed Aug 16, 2018

View reviewed changes

jdalton approved these changes Aug 16, 2018

View reviewed changes

doc: md references should be alphabetical

6e6192b

vsemozhetbyt removed the blocked PRs that are blocked by other issues or PRs. label Aug 16, 2018

joyeecheung mentioned this pull request Aug 17, 2018

CI Failures: 20180817 nodejs/reliability#14

Closed

vsemozhetbyt closed this Aug 18, 2018

SirR4T deleted the addCliDocForLoader branch August 22, 2018 05:31

targos mentioned this pull request Sep 5, 2018

Release proposal: v10.10.0 #22716

Merged

This was referenced Sep 6, 2018

2018-09-06 Version 10.10.0 (Current) @targos beevelop/docker-android-nodejs#236

Closed

2018-09-06 Version 10.10.0 (Current) @targos beevelop/docker-nodejs#221

Closed

aduh95 mentioned this pull request Apr 18, 2022

[v16.x backport] esm: add HTTPS imports and unflag JSON modules #42726

Closed

aliceUnhinged613 mentioned this pull request May 16, 2022

[Snyk] Security upgrade gatsby from 2.18.18 to 2.19.36 aliceUnhinged613/node#58

Open

This was referenced May 16, 2022

[Snyk] Security upgrade gatsby from 2.18.18 to 2.19.36 137717unity/node#57

Open

[Snyk] Security upgrade gatsby from 2.18.18 to 2.19.36 Rednickle/node-1#49

Open

kissofire mentioned this pull request May 16, 2022

[Snyk] Security upgrade gatsby from 2.13.73 to 2.19.36 kissofire/node#67

Open

wedataintelligence mentioned this pull request May 16, 2022

[Snyk] Security upgrade gatsby from 2.18.18 to 2.19.36 wedataintelligence/node#33

Open

ron-a20 mentioned this pull request May 16, 2022

[Snyk] Security upgrade gatsby from 2.18.18 to 2.19.36 ron-a20/node#66

Open

molllyn1 mentioned this pull request May 17, 2022

[Snyk] Security upgrade gatsby from 2.18.18 to 2.19.36 molllyn1/node#48

Open

               [debugger]: debugger.html
               [emit_warning]: process.html#process_process_emitwarning_warning_type_code_ctor
               [libuv threadpool documentation]: http://docs.libuv.org/en/latest/threadpool.html
+              [experimental ECMAScript Module]: esm.html#esm_loader_hooks

doc: add doc for --loader option by SirR4T · Pull Request #22104 · nodejs/node · GitHub

Uh oh!

Uh oh!

doc: add doc for --loader option - #22104

doc: add doc for --loader option#22104SirR4T wants to merge 6 commits intonodejs:masternodejs/node:masterfrom SirR4T:addCliDocForLoaderSirR4T/node:addCliDocForLoaderCopy head branch name to clipboard

Conversation

SirR4T commented Aug 3, 2018

Checklist

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vsemozhetbyt commented Aug 3, 2018

Uh oh!

vsemozhetbyt commented Aug 3, 2018

Uh oh!

SirR4T commented Aug 6, 2018

Uh oh!

vsemozhetbyt commented Aug 6, 2018

Uh oh!

vsemozhetbyt commented Aug 6, 2018

Uh oh!

SirR4T commented Aug 6, 2018

Uh oh!

vsemozhetbyt commented Aug 6, 2018

Uh oh!

This comment has been minimized.

This comment has been minimized.

vsemozhetbyt commented Aug 8, 2018

Uh oh!

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

vsemozhetbyt commented Aug 9, 2018

Uh oh!

sagirk left a comment

Choose a reason for hiding this comment

Uh oh!

vsemozhetbyt commented Aug 10, 2018

Uh oh!

SirR4T commented Aug 10, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vsemozhetbyt commented Aug 10, 2018

Uh oh!

SirR4T commented Aug 16, 2018

Uh oh!

jdalton commented Aug 16, 2018

Uh oh!

SirR4T commented Aug 16, 2018

Uh oh!

vsemozhetbyt Aug 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SirR4T Aug 16, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vsemozhetbyt commented Aug 16, 2018

Uh oh!

doc: add doc for --loader option#22104
SirR4T wants to merge 6 commits into
nodejs:masterfrom
SirR4T:addCliDocForLoader

SirR4T commented Aug 10, 2018 •
edited
Loading

vsemozhetbyt Aug 16, 2018 •
edited
Loading

SirR4T Aug 16, 2018 •
edited
Loading

vsemozhetbyt commented Aug 16, 2018 •
edited
Loading