[12.x] modules backports by guybedford · Pull Request #35385 · nodejs/node

and others added 13 commits

September 27, 2020 19:41
PR-URL: nodejs#34117
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
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#32261
Fixes: nodejs#31595
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
PR-URL: nodejs#34605
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: nodejs#34637
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
PR-URL: nodejs#34744
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>
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 <guybedford@gmail.com>

PR-URL: nodejs#34595
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Mary Marchini <oss@mmarchini.me>
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 <anna@addaleax.net>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>
Refs: nodejs#34765

PR-URL: nodejs#34795
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>
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 <guybedford@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>
PR-URL: nodejs#35117
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: nodejs#34951
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>
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 <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
PR-URL: nodejs#34718
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>

@addaleax addaleax added the esm

Issues and PRs related to the ECMAScript Modules implementation.

label

Sep 28, 2020

codebytere

codebytere pushed a commit that referenced this pull request

Oct 1, 2020
PR-URL: #34117
Backport-PR-URL: #35385
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>

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: #35385
Fixes: #31595
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>

codebytere pushed a commit that referenced this pull request

Oct 1, 2020
PR-URL: #34605
Backport-PR-URL: #35385
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

codebytere pushed a commit that referenced this pull request

Oct 1, 2020
PR-URL: #34637
Backport-PR-URL: #35385
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Geoffrey Booth <webmaster@geoffreybooth.com>
Reviewed-By: Jan Krems <jan.krems@gmail.com>

codebytere pushed a commit that referenced this pull request

Oct 1, 2020
PR-URL: #34744
Backport-PR-URL: #35385
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: James M Snell <jasnell@gmail.com>

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 <guybedford@gmail.com>

PR-URL: #34595
Backport-PR-URL: #35385
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Mary Marchini <oss@mmarchini.me>

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 <anna@addaleax.net>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Shelley Vohr <codebytere@gmail.com>

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 <matteo.collina@gmail.com>
Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de>
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Rich Trott <rtrott@gmail.com>

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 <guybedford@gmail.com>
Reviewed-By: Denys Otrishko <shishugi@gmail.com>

codebytere pushed a commit that referenced this pull request

Oct 1, 2020
PR-URL: #35117
Backport-PR-URL: #35385
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

codebytere pushed a commit that referenced this pull request

Oct 1, 2020
PR-URL: #34951
Backport-PR-URL: #35385
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Derek Lewis <DerekNonGeneric@inf.is>

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 <matteo.collina@gmail.com>
Reviewed-By: Anna Henningsen <anna@addaleax.net>
Reviewed-By: Myles Borins <myles.borins@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>

codebytere pushed a commit that referenced this pull request

Oct 1, 2020
PR-URL: #34718
Backport-PR-URL: #35385
Reviewed-By: Jan Krems <jan.krems@gmail.com>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>