Skip to content

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

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
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

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

richardlau commented Jul 23, 2021

@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

nodejs-github-bot commented Jul 23, 2021

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

nodejs-github-bot commented Jul 28, 2021

CI: https://ci.nodejs.org/job/node-test-pull-request/39318/
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 28, 2021

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

richardlau commented Jul 28, 2021

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

Labels

esm Issues and PRs related to the ECMAScript Modules implementation. needs-ci PRs that need a full CI run.

5 participants

@aduh95 @richardlau @ljharb @nodejs-github-bot @guybedford