doc: update and improve the CommonJS page#57055
doc: update and improve the CommonJS page#57055dario-piotrowicz wants to merge 1 commit intonodejs:mainfrom
Conversation
|
Review requested:
|
c9a9574 to
bf9e61c
Compare
| 2. Else, load X/index.js as an CommonJS module. STOP. | ||
| 2. If X/index.json is a file, parse X/index.json to a JavaScript object. STOP | ||
| 3. If X/index.node is a file, load X/index.node as binary addon. STOP | ||
| MAYBE_DETECT_AND_LOAD(X) |
There was a problem hiding this comment.
Question: here and in other parts of this pseudo-code there are various anchors, those are rendered as plain text and not actual links, is this intentional?
| * <a id="modules_new_sourcemap_payload" href="module.html#new-sourcemappayload">`new SourceMap(payload)`</a> | ||
| * <a id="modules_sourcemap_payload" href="module.html#sourcemappayload">`sourceMap.payload`</a> | ||
| * <a id="modules_sourcemap_findentry_linenumber_columnnumber" href="module.html#sourcemapfindentrylinenumber-columnnumber">`sourceMap.findEntry(lineNumber, columnNumber)`</a> |
There was a problem hiding this comment.
I've aligned these with the rest of the anchor tags, but I don't think that these (and the ones above) are correct.
This comment makes me think that these should not be visible:
Line 1268 in cc7018e
And indeed they (to me at least) look totally out of place in the rendered page:
@aduh95 from the git history it seems like you placed these here (#34747), could you advice whether these are ok to stay, be hidden or removed? (or converted to i tags and moved in the various sections of this page (like here))
There was a problem hiding this comment.
PS: also the new SourceMap(payload) and sourceMap.findEntry(lineNumber, columnNumber) links are actually broken
These changes: - update outdated info (`module.buildinModules` includes all built-ins) - fix various typos - improve consistency - polish and make more clear some sentences
bf9e61c to
b3c1f55
Compare
|
This is very hard to review, and will be a nightmare to backport. Could you send several PRs, each focusing on a specific point, please? |
oh... ok 🥲👍 |
|
It's too cumbersome to move everything to simple PRs... I think I will then drop a bunch of improvements here 😓 (I don't think it makes sense to open a hundred small/trivial PRs for updating a single page) |
These changes:
module.buildinModulesincludes all built-ins since 23.5.0)Fixes #51780