doc: update module customization hooks docs #49265
Conversation
GeoffreyBooth
added
doc
|
Review requested:
|
GeoffreyBooth
force-pushed
the
update-module-customization-hooks-docs
branch
2 times, most recently
from
e96232b
to
0a327aa
Compare
|
Maybe we could address #49282 as part of this PR? |
GeoffreyBooth
force-pushed
the
update-module-customization-hooks-docs
branch
from
5686941
to
aa414fa
Compare
How, exactly? The examples already show the functions’ signatures as being async. Edit: The |
doc/api/module.md
Outdated
| `second.mjs` define a `resolve` hook, both will be called, in the order they | ||
| were registered. The same applies to all the other hooks. | ||
|
|
||
| ### Communication between main and hooks threads |
There was a problem hiding this comment.
Should we keep calling it the loader thread?
There was a problem hiding this comment.
I don't think so. It's the thread where the hooks run. One term is better than two.
There was a problem hiding this comment.
Orchestrator/orchestration threads?
There was a problem hiding this comment.
I really don't like "hooks threads", especially if the goal is to make room for hooks in other core modules. Let's keep "loader thread" until we find a better name?
| ### Communication between main and hooks threads | |
| ### Communication between main and loader threads |
There was a problem hiding this comment.
Customization thread?
I wasn't assuming we'd need a thread like this (or use this one) for other systems, but who knows?
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
Co-authored-by: Jacob Smith <3012099+JakobJingleheimer@users.noreply.github.com>
GeoffreyBooth
force-pushed
the
update-module-customization-hooks-docs
branch
from
4937898
to
230124d
Compare
doc/api/module.md
Outdated
| `second.mjs` define a `resolve` hook, both will be called, in the order they | ||
| were registered. The same applies to all the other hooks. | ||
|
|
||
| ### Communication between main and hooks threads |
There was a problem hiding this comment.
I really don't like "hooks threads", especially if the goal is to make room for hooks in other core modules. Let's keep "loader thread" until we find a better name?
| ### Communication between main and hooks threads | |
| ### Communication between main and loader threads |
| `resolve`); if `resolve` provides a `format`, a custom `load` hook is required | ||
| even if only to pass the value to the Node.js default `load` hook. |
There was a problem hiding this comment.
While we're editing this, we should consider rephrase that, I don't understand what it means.
Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com>
GeoffreyBooth
added
commit-queue
nodejs-github-bot
removed
the
commit-queue
|
Landed in 33710e7 |
PR-URL: #49265 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs#49265 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs#49265 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs/node#49265 Backport-PR-URL: nodejs/node#50669 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
PR-URL: nodejs/node#49265 Backport-PR-URL: nodejs/node#50669 Reviewed-By: Jacob Smith <jacob@frende.me> Reviewed-By: Antoine du Hamel <duhamelantoine1995@gmail.com>
This PR builds on and follows up #49261.
The aim here was to implement most of this to-do list. In particular:
I hopefully removed all references to the “Loaders API” and similar nomenclature, in favor of “module customization hooks” or “hooks” or similar.
I moved most of the “how to use this API” text out of the
module.register()section and into a new section “Enabling” under the “Customization Hooks” heading.I removed all references to
--experimental-loaderexcept incli.md, rewriting to recommend--importwithregister()instead. We’re considering removing the--experimental-loaderflag, but for now at least we want to discourage its use. (We anticipate that most third-party libraries likets-nodewill need code on the main thread, which--importwithregister()can enable but--experimental-loadercannot, so people should get familiar with the more powerful approach and we don’t need a secondary, less capable method.)I added CommonJS versions of all code samples that could support them (so anything running on the main thread, since the hooks module needs to export specific named functions).
I classified the overall Module Customization Hooks API as stability level “1.1 - Active development” and the
resolveandloadhooks as “1.2 - Release candidate,” andglobalPreloadas “1.0 - Early development” (since I can’t mark it as deprecated, but its end is near whenever esm: removeglobalPreloadhook (superseded byinitialize) #49144 lands).I added some mentions that parts of this work for CommonJS modules and
requireas well.The CoffeeScript example seemed to have gotten out of date, so I rewrote it to how I think it should be now that CommonJS is supported.
Miscellaneous cleanup.
What’s left for follow-up PRs:
Remove the
globalPreloadsection as part of esm: removeglobalPreloadhook (superseded byinitialize) #49144.Remove the mention of
--experimental-loaderfromcli.md?Update
--experimental-loaderto print a warning that it may be removed in the future, and users should use--importwithregisterinstead; this warning could even generate example code that shows--importwith a data URL that includes the value of what was passed to--experimental-loader(similar to how we have a fancy error message with suggested new code for unknown named exports from CommonJS modules).Also fixes #49282.
cc @nodejs/loaders @nodejs/modules @aduh95 @JakobJingleheimer