Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[v12.x backport] module: fix legacy node specifier resolution to resolve "main" field #39497

Closed
wants to merge 1 commit into from
Closed

[v12.x backport] module: fix legacy node specifier resolution to resolve "main" field #39497

wants to merge 1 commit into from
+27 −6

Conversation

aduh95
Copy link
Contributor

@aduh95 aduh95 commented Jul 23, 2021

PR-URL: #38979
Fixes: #32103
Fixes: #38739
Reviewed-By: Bradley Farias bradley.meck@gmail.com
Reviewed-By: Guy Bedford guybedford@gmail.com

@github-actions github-actions bot added esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run. v12.x labels Jul 23, 2021
@richardlau
Copy link
Member

@aduh95 Test failures
https://github.com/nodejs/node/pull/39497/checks?check_run_id=3141958951#step:5:71

=== release test-esm-specifiers ===
Path: es-module/test-esm-specifiers
--- stderr ---
internal/process/esm_loader.js:74
    internalBinding('errors').triggerUncaughtException(
                              ^

TypeError [ERR_INVALID_URL]: Invalid URL: /home/runner/work/node/node/test/fixtures/es-module-specifiers/package-type-commonjs/package.json
    at onParseError (internal/url.js:258:9)
    at new URL (internal/url.js:334:5)
    at fileURLToPath (internal/url.js:1350:12)
    at fileExists (internal/modules/esm/resolve.js:174:22)
    at resolveDirectoryEntry (internal/modules/esm/resolve.js:244:7)
    at finalizeResolution (internal/modules/esm/resolve.js:269:14)
    at moduleResolve (internal/modules/esm/resolve.js:708:10)
    at Loader.defaultResolve [as _resolve] (internal/modules/esm/resolve.js:798:11)
    at Loader.resolve (internal/modules/esm/loader.js:100:40)
    at Loader.getModuleJob (internal/modules/esm/loader.js:246:28) {
  input: '/home/runner/work/node/node/test/fixtures/es-module-specifiers/package-type-commonjs/package.json',
  code: 'ERR_INVALID_URL'
}
Command: out/Release/node --experimental-specifier-resolution=node /home/runner/work/node/node/test/es-module/test-esm-specifiers.mjs
=== release test-esm-specifiers-legacy-flag ===
Path: es-module/test-esm-specifiers-legacy-flag
--- stderr ---
internal/process/esm_loader.js:74
    internalBinding('errors').triggerUncaughtException(
                              ^

TypeError [ERR_INVALID_URL]: Invalid URL: /home/runner/work/node/node/test/fixtures/es-module-specifiers/package-type-commonjs/package.json
    at onParseError (internal/url.js:258:9)
    at new URL (internal/url.js:334:5)
    at fileURLToPath (internal/url.js:1350:12)
    at fileExists (internal/modules/esm/resolve.js:174:22)
    at resolveDirectoryEntry (internal/modules/esm/resolve.js:244:7)
    at finalizeResolution (internal/modules/esm/resolve.js:269:14)
    at moduleResolve (internal/modules/esm/resolve.js:708:10)
    at Loader.defaultResolve [as _resolve] (internal/modules/esm/resolve.js:798:11)
    at Loader.resolve (internal/modules/esm/loader.js:100:40)
    at Loader.getModuleJob (internal/modules/esm/loader.js:246:28) {
  input: '/home/runner/work/node/node/test/fixtures/es-module-specifiers/package-type-commonjs/package.json',
  code: 'ERR_INVALID_URL'
}
Command: out/Release/node --es-module-specifier-resolution=node /home/runner/work/node/node/test/es-module/test-esm-specifiers-legacy-flag.mjs

@aduh95 aduh95 force-pushed the backport-node-specifier-resolver-fix branch from 69b2eae to 5f22ec4 Compare July 23, 2021 11:23
@ljharb
Copy link
Member

ljharb commented Jul 23, 2021

cc @nodejs/modules

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/39244/

@aduh95
module: fix legacy node specifier resolution to resolve "main" field
43724ac 
PR-URL: nodejs#38979
Fixes: nodejs#32103
Fixes: nodejs#38739
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
@aduh95 aduh95 force-pushed the backport-node-specifier-resolver-fix branch from 5f22ec4 to 43724ac Compare July 28, 2021 17:15
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/39318/

@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/39321/

richardlau pushed a commit that referenced this pull request Jul 28, 2021
@aduh95 @richardlau
module: fix legacy node specifier resolution to resolve "main" field
f0bb3d2 
PR-URL: #38979
Backport-PR-URL: #39497
Fixes: #32103
Fixes: #38739
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
@richardlau
Copy link
Member

Landed in f0bb3d2

@richardlau richardlau closed this Jul 28, 2021
richardlau pushed a commit that referenced this pull request Jul 28, 2021
@aduh95 @richardlau
module: fix legacy node specifier resolution to resolve "main" field
3e4bc1b 
PR-URL: #38979
Backport-PR-URL: #39497
Fixes: #32103
Fixes: #38739
Reviewed-By: Bradley Farias <bradley.meck@gmail.com>
Reviewed-By: Guy Bedford <guybedford@gmail.com>
@aduh95 aduh95 deleted the backport-node-specifier-resolver-fix branch July 28, 2021 22:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@richardlau richardlau richardlau approved these changes

@guybedford guybedford guybedford approved these changes

Assignees
No one assigned
Labels
esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@aduh95 @richardlau @ljharb @nodejs-github-bot @guybedford