[12.x] modules backports#35385
Closed
[12.x] modules backports #35385
Uh oh!
There was an error while loading. Please reload this page.
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters. Learn more about bidirectional Unicode characters
PR-URL: nodejs#34117 Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Bradley Farias <[email protected]>
Load self referential modules from the repl and using the preload flag `-r`. In both cases the base path used for resolution is the current `process.cwd()`. Also fixes an internal cycle bug in the REPL exports resolution. PR-URL: nodejs#32261Fixes: nodejs#31595 Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Jan Krems <[email protected]>
PR-URL: nodejs#34605 Reviewed-By: Bradley Farias <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: nodejs#34637 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Jan Krems <[email protected]>
PR-URL: nodejs#34744 Reviewed-By: Jan Krems <[email protected]> Reviewed-By: James M Snell <[email protected]>
This patch converts the "read package scope" algorithm's while loop into a do-while loop enabling items at the filesystem root dir to be considered within the scope of a sibling package.json also at the filesystem root dir. Fixes: nodejs#33438 Co-authored-by: Guy Bedford <[email protected]> PR-URL: nodejs#34595 Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Mary Marchini <[email protected]>
Old versions of mocha break after nodejs#34637. This was a bug in mocha, but since this is a widely used module we can expect ecosystem breakage until modules are updated to the latest version of mocha. Drop the conflicting `-u` alias -- we can potentially bring it back once modules have been updated. PR-URL: nodejs#34935 Refs: mochajs/mocha#4417 Refs: nodejs#34637 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Beth Griggs <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Shelley Vohr <[email protected]>
Refs: nodejs#34765 PR-URL: nodejs#34795 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Bradley Farias <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]>
I know it just got modified to include new information, but this shortens the message a bit without (I hope) losing clarity or meaning. PR-URL: nodejs#34836 Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Denys Otrishko <[email protected]>
PR-URL: nodejs#35117 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: nodejs#34951 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Derek Lewis <[email protected]>
The node process crashes when trying to parse a multiline import statement for named exports of a CommonJS module: TypeError: Cannot read property '0' of null at ModuleJob._instantiate (internal/modules/esm/module_job.js:112:77) at async ModuleJob.run (internal/modules/esm/module_job.js:137:5) at async Loader.import (internal/modules/esm/loader.js:165:24) at async rejects.name (file:///***/node/test/es-module/test-esm-cjs-named-error.mjs:56:3) at async waitForActual (assert.js:721:5) at async rejects (assert.js:830:25), The reason is that the regexp that is currently used to decorate the original error fails for multi line import statements. Unfortunately the undecorated error stack only contains the single line which causes the import to fail: file:///***/node/test/fixtures/es-modules/package-cjs-named-error/multi-line.mjs:2 comeOn, ^^^^^^ SyntaxError: The requested module './fail.cjs' does not provide an export named 'comeOn' at ModuleJob._instantiate (internal/modules/esm/module_job.js:98:21) at async ModuleJob.run (internal/modules/esm/module_job.js:141:5) at async Loader.import (internal/modules/esm/loader.js:165:24) at async rejects.name (file:///***/node/test/es-module/test-esm-cjs-named-error.mjs:56:3) at async waitForActual (assert.js:721:5) at async rejects (assert.js:830:25) Hence, for multiline import statements we cannot create an equivalent piece of code that uses default import followed by an object destructuring assignment. In any case the node process should definitely not crash. So until we have a more sophisticated way of extracting the entire problematic multiline import statement, show the code example only for single-line imports where the current regexp approach works well. Refs: nodejs#35259 PR-URL: nodejs#35275 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Rich Trott <[email protected]>
PR-URL: nodejs#34718 Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Collaborator
nodejs-github-bot commented Sep 28, 2020
Review requested:
|
nodejs-github-bot added lib / src 
4 tasks
addaleaxforce-pushed the v12.x-staging branch from 52627bc to 2f67e99Compare
github-actionsbot removed the request-ci Collaborator
nodejs-github-bot commented Sep 28, 2020
addaleax added the esm Collaborator
nodejs-github-bot commented Sep 28, 2020
4 tasks
hybrist approved these changes Oct 1, 2020
Contributor
hybrist left a comment
hybrist left a comment There was a problem hiding this comment.
RSLGTM (skimmed the changes and they look like I'd expect)
codebytere approved these changes Oct 1, 2020
Member
codebytere commented Oct 1, 2020
Landed in b7be751...2f3ffc0 |
codebytere pushed a commit that referenced this pull request Oct 1, 2020
PR-URL: #34117 Backport-PR-URL: #35385 Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Bradley Farias <[email protected]>
codebytere pushed a commit that referenced this pull request Oct 1, 2020
Load self referential modules from the repl and using the preload flag `-r`. In both cases the base path used for resolution is the current `process.cwd()`. Also fixes an internal cycle bug in the REPL exports resolution. PR-URL: #32261 Backport-PR-URL: #35385Fixes: #31595 Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Jan Krems <[email protected]>
codebytere pushed a commit that referenced this pull request Oct 1, 2020
PR-URL: #34605 Backport-PR-URL: #35385 Reviewed-By: Bradley Farias <[email protected]> Reviewed-By: Rich Trott <[email protected]>
codebytere pushed a commit that referenced this pull request Oct 1, 2020
PR-URL: #34637 Backport-PR-URL: #35385 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Geoffrey Booth <[email protected]> Reviewed-By: Jan Krems <[email protected]>
codebytere pushed a commit that referenced this pull request Oct 1, 2020
PR-URL: #34744 Backport-PR-URL: #35385 Reviewed-By: Jan Krems <[email protected]> Reviewed-By: James M Snell <[email protected]>
codebytere pushed a commit that referenced this pull request Oct 1, 2020
This patch converts the "read package scope" algorithm's while loop into a do-while loop enabling items at the filesystem root dir to be considered within the scope of a sibling package.json also at the filesystem root dir. Fixes: #33438 Co-authored-by: Guy Bedford <[email protected]> PR-URL: #34595 Backport-PR-URL: #35385 Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Mary Marchini <[email protected]>
codebytere pushed a commit that referenced this pull request Oct 1, 2020
Old versions of mocha break after #34637. This was a bug in mocha, but since this is a widely used module we can expect ecosystem breakage until modules are updated to the latest version of mocha. Drop the conflicting `-u` alias -- we can potentially bring it back once modules have been updated. PR-URL: #34935 Backport-PR-URL: #35385 Refs: mochajs/mocha#4417 Refs: #34637 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Beth Griggs <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Colin Ihrig <[email protected]> Reviewed-By: Shelley Vohr <[email protected]>
codebytere pushed a commit that referenced this pull request Oct 1, 2020
Refs: #34765 PR-URL: #34795 Backport-PR-URL: #35385 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Ruben Bridgewater <[email protected]> Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Bradley Farias <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Rich Trott <[email protected]>
codebytere pushed a commit that referenced this pull request Oct 1, 2020
I know it just got modified to include new information, but this shortens the message a bit without (I hope) losing clarity or meaning. PR-URL: #34836 Backport-PR-URL: #35385 Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Denys Otrishko <[email protected]>
codebytere pushed a commit that referenced this pull request Oct 1, 2020
PR-URL: #35117 Backport-PR-URL: #35385 Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Michael Dawson <[email protected]> Reviewed-By: Rich Trott <[email protected]>
codebytere pushed a commit that referenced this pull request Oct 1, 2020
PR-URL: #34951 Backport-PR-URL: #35385 Reviewed-By: Benjamin Gruenbaum <[email protected]> Reviewed-By: Derek Lewis <[email protected]>
codebytere pushed a commit that referenced this pull request Oct 1, 2020
The node process crashes when trying to parse a multiline import statement for named exports of a CommonJS module: TypeError: Cannot read property '0' of null at ModuleJob._instantiate (internal/modules/esm/module_job.js:112:77) at async ModuleJob.run (internal/modules/esm/module_job.js:137:5) at async Loader.import (internal/modules/esm/loader.js:165:24) at async rejects.name (file:///***/node/test/es-module/test-esm-cjs-named-error.mjs:56:3) at async waitForActual (assert.js:721:5) at async rejects (assert.js:830:25), The reason is that the regexp that is currently used to decorate the original error fails for multi line import statements. Unfortunately the undecorated error stack only contains the single line which causes the import to fail: file:///***/node/test/fixtures/es-modules/package-cjs-named-error/multi-line.mjs:2 comeOn, ^^^^^^ SyntaxError: The requested module './fail.cjs' does not provide an export named 'comeOn' at ModuleJob._instantiate (internal/modules/esm/module_job.js:98:21) at async ModuleJob.run (internal/modules/esm/module_job.js:141:5) at async Loader.import (internal/modules/esm/loader.js:165:24) at async rejects.name (file:///***/node/test/es-module/test-esm-cjs-named-error.mjs:56:3) at async waitForActual (assert.js:721:5) at async rejects (assert.js:830:25) Hence, for multiline import statements we cannot create an equivalent piece of code that uses default import followed by an object destructuring assignment. In any case the node process should definitely not crash. So until we have a more sophisticated way of extracting the entire problematic multiline import statement, show the code example only for single-line imports where the current regexp approach works well. Refs: #35259 PR-URL: #35275 Backport-PR-URL: #35385 Reviewed-By: Matteo Collina <[email protected]> Reviewed-By: Anna Henningsen <[email protected]> Reviewed-By: Myles Borins <[email protected]> Reviewed-By: Rich Trott <[email protected]>
codebytere pushed a commit that referenced this pull request Oct 1, 2020
PR-URL: #34718 Backport-PR-URL: #35385 Reviewed-By: Jan Krems <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
7 tasks
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment
12 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This includes the latest modules backports from 14 to 12, which should then ensure that #35249 can land cleanly as well while getting the latest support for the modules package.json fields for compatibility.
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes