doc: add doc for --loader option #22104

SirR4T · 2018-08-03T09:28:01Z

Checklist

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

jasnell · 2018-08-03T22:35:12Z

doc/api/cli.md

+added: v9.0.0
+-->
+
+Specify a customer loader, to load [ECMAScript Modules][].


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.

vsemozhetbyt · 2018-08-03T22:45:38Z

doc/api/cli.md

@@ -687,3 +694,4 @@ greater than `4` (its current default value). For more information, see the
 [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
+[ECMAScript Modules]: esm.html#loader_hooks


esm.html#esm_loader_hooks?

thanks! fixed now.

vsemozhetbyt · 2018-08-03T22:46:41Z

cc @nodejs/modules-active-members?

vsemozhetbyt · 2018-08-03T22:52:13Z

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

SirR4T · 2018-08-06T05:21:36Z

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

vsemozhetbyt · 2018-08-06T05:36:07Z

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

vsemozhetbyt · 2018-08-06T05:37:09Z

Linter issue:

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

SirR4T · 2018-08-06T05:53:40Z

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

vsemozhetbyt · 2018-08-06T06:25:20Z

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'

vsemozhetbyt · 2018-08-08T16:54:19Z

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

vsemozhetbyt · 2018-08-09T21:22:23Z

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.

sagirk

LGTM! 🎉

vsemozhetbyt · 2018-08-10T05:46:23Z

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/

SirR4T · 2018-08-10T06:06:33Z

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

vsemozhetbyt · 2018-08-10T06:14:43Z

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

SirR4T · 2018-08-16T05:44:59Z

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

jdalton · 2018-08-16T05:52:29Z

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

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

SirR4T · 2018-08-16T06:26:46Z

thanks, @jdalton , fixed.

vsemozhetbyt · 2018-08-16T06:35:56Z

doc/api/cli.md

@@ -687,3 +694,4 @@ greater than `4` (its current default value). For more information, see the
 [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


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.

vsemozhetbyt · 2018-08-16T16:18:11Z

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

vsemozhetbyt · 2018-08-16T18:01:13Z

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/

vsemozhetbyt · 2018-08-18T08:17:11Z

Landed in 9d6619e
Thank you!

PR-URL: #22104 Fixes: #21230 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: John-David Dalton <[email protected]>

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

targos pushed a commit that referenced this pull request Aug 19, 2018

doc: add doc for --loader option

c76fb12

PR-URL: #22104 Fixes: #21230 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: John-David Dalton <[email protected]>

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

targos pushed a commit that referenced this pull request Sep 3, 2018

doc: add doc for --loader option

e644088

PR-URL: #22104 Fixes: #21230 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: James M Snell <[email protected]> Reviewed-By: John-David Dalton <[email protected]>

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

doc: add doc for --loader option #22104

doc: add doc for --loader option #22104

Conversation

SirR4T commented Aug 3, 2018

Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vsemozhetbyt commented Aug 3, 2018

vsemozhetbyt commented Aug 3, 2018

SirR4T commented Aug 6, 2018

vsemozhetbyt commented Aug 6, 2018

vsemozhetbyt commented Aug 6, 2018

SirR4T commented Aug 6, 2018

vsemozhetbyt commented Aug 6, 2018

This comment has been minimized.

This comment has been minimized.

vsemozhetbyt commented Aug 8, 2018

This comment has been minimized.

This comment has been minimized.

This comment has been minimized.

vsemozhetbyt commented Aug 9, 2018

sagirk left a comment

Choose a reason for hiding this comment

vsemozhetbyt commented Aug 10, 2018

SirR4T commented Aug 10, 2018 • edited Loading

vsemozhetbyt commented Aug 10, 2018

SirR4T commented Aug 16, 2018

jdalton commented Aug 16, 2018

SirR4T commented Aug 16, 2018

vsemozhetbyt Aug 16, 2018 • edited Loading

Choose a reason for hiding this comment

SirR4T Aug 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vsemozhetbyt commented Aug 16, 2018

vsemozhetbyt commented Aug 16, 2018 • edited Loading

vsemozhetbyt commented Aug 18, 2018

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