Skip to content

module: fix discrepancy between .ts and .js #54461

New issue

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

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

Already on GitHub? # to your account

Merged

Conversation

marco-ippolito
Copy link
Member

Fixes: #54457

This fixes the discrepancy with the .js behavior

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/loaders
  • @nodejs/typescript

@nodejs-github-bot nodejs-github-bot added esm Issues and PRs related to the ECMAScript Modules implementation. module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run. labels Aug 20, 2024
Copy link
Contributor

@ShogunPanda ShogunPanda left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Copy link

codecov bot commented Aug 20, 2024

Codecov Report

Attention: Patch coverage is 98.38710% with 1 line in your changes missing coverage. Please review.

Project coverage is 87.31%. Comparing base (ef4bdbf) to head (af4f6c9).
Report is 779 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/modules/helpers.js 98.11% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #54461      +/-   ##
==========================================
- Coverage   87.32%   87.31%   -0.01%     
==========================================
  Files         648      648              
  Lines      182383   182389       +6     
  Branches    34991    34987       -4     
==========================================
+ Hits       159260   159262       +2     
- Misses      16384    16390       +6     
+ Partials     6739     6737       -2     
Files with missing lines Coverage Δ
lib/internal/modules/esm/get_format.js 88.88% <100.00%> (-1.59%) ⬇️
lib/internal/modules/esm/translators.js 91.49% <100.00%> (-0.56%) ⬇️
lib/internal/modules/helpers.js 97.61% <98.11%> (+0.06%) ⬆️

... and 29 files with indirect coverage changes

@marco-ippolito marco-ippolito added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 20, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 20, 2024
@nodejs-github-bot
Copy link
Collaborator

@marco-ippolito marco-ippolito added the request-ci Add this label to start a Jenkins CI on a PR. label Aug 20, 2024
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Aug 20, 2024
@nodejs-github-bot
Copy link
Collaborator

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@cjihrig
Copy link
Contributor

cjihrig commented Aug 20, 2024

I can confirm this fixes the issue I was seeing. Thank you for the quick fix.

@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@marco-ippolito marco-ippolito added commit-queue Add this label to land a pull request using GitHub Actions. author ready PRs that have at least one approval, no pending requests for changes, and a CI started. strip-types Issues or PRs related to strip-types support labels Aug 21, 2024
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Aug 22, 2024
@nodejs-github-bot nodejs-github-bot merged commit e35902c into nodejs:main Aug 22, 2024
74 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in e35902c

@RafaelGSS RafaelGSS added the backport-requested-v22.x PRs awaiting manual backport to the v22.x-staging branch. label Aug 25, 2024
@RafaelGSS
Copy link
Member

This commit didn't land cleanly on v22.x-staging. Can you create a manual backport?

marco-ippolito added a commit to marco-ippolito/node that referenced this pull request Aug 26, 2024
PR-URL: nodejs#54461
Fixes: nodejs#54457
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
marco-ippolito added a commit to marco-ippolito/node that referenced this pull request Aug 26, 2024
PR-URL: nodejs#54461
Fixes: nodejs#54457
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
@avivkeller avivkeller added backport-open-v22.x Indicate that the PR has an open backport and removed backport-requested-v22.x PRs awaiting manual backport to the v22.x-staging branch. labels Aug 26, 2024
marco-ippolito added a commit to marco-ippolito/node that referenced this pull request Aug 31, 2024
PR-URL: nodejs#54461
Fixes: nodejs#54457
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
marco-ippolito added a commit to marco-ippolito/node that referenced this pull request Sep 2, 2024
PR-URL: nodejs#54461
Fixes: nodejs#54457
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
marco-ippolito added a commit to marco-ippolito/node that referenced this pull request Sep 12, 2024
PR-URL: nodejs#54461
Fixes: nodejs#54457
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
marco-ippolito added a commit to marco-ippolito/node that referenced this pull request Sep 16, 2024
PR-URL: nodejs#54461
Fixes: nodejs#54457
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
marco-ippolito added a commit to marco-ippolito/node that referenced this pull request Sep 16, 2024
PR-URL: nodejs#54461
Fixes: nodejs#54457
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
marco-ippolito added a commit to marco-ippolito/node that referenced this pull request Sep 16, 2024
PR-URL: nodejs#54461
Fixes: nodejs#54457
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
marco-ippolito added a commit to marco-ippolito/node that referenced this pull request Sep 17, 2024
PR-URL: nodejs#54461
Fixes: nodejs#54457
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
marco-ippolito added a commit to marco-ippolito/node that referenced this pull request Sep 17, 2024
PR-URL: nodejs#54461
Fixes: nodejs#54457
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
marco-ippolito added a commit that referenced this pull request Sep 18, 2024
PR-URL: #54461
Backport-PR-URL: #54566
Fixes: #54457
Reviewed-By: Paolo Insogna <paolo@cowtech.it>
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
Reviewed-By: Zeyu "Alex" Yang <himself65@outlook.com>
@targos targos added backported-to-v22.x PRs backported to the v22.x-staging branch. and removed backport-open-v22.x Indicate that the PR has an open backport labels Oct 5, 2024
@aduh95 aduh95 mentioned this pull request Oct 9, 2024
@marco-ippolito marco-ippolito added the dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. label Nov 16, 2024
codebytere added a commit to electron/electron that referenced this pull request Nov 20, 2024
codebytere added a commit to electron/electron that referenced this pull request Nov 20, 2024
codebytere added a commit to electron/electron that referenced this pull request Nov 21, 2024
codebytere added a commit to electron/electron that referenced this pull request Dec 3, 2024
codebytere added a commit to electron/electron that referenced this pull request Dec 4, 2024
codebytere added a commit to electron/electron that referenced this pull request Jan 22, 2025
samuelmaddock pushed a commit to electron/electron that referenced this pull request Jan 22, 2025
* chore: bump node in DEPS to v22.11.0

* src: move evp stuff to ncrypto

nodejs/node#54911

* crypto: add Date fields for validTo and validFrom

nodejs/node#54159

* module: fix discrepancy between .ts and .js

nodejs/node#54461

* esm: do not interpret "main" as a URL

nodejs/node#55003

* src: modernize likely/unlikely hints

nodejs/node#55155

* chore: update patch indices

* crypto: add validFromDate and validToDate fields to X509Certificate

nodejs/node#54159

* chore: fixup perfetto patch

* fix: clang warning in simdjson

* src: add receiver to fast api callback methods

nodejs/node#54408

* chore: fixup revert patch

* fixup! esm: do not interpret "main" as a URL

* fixup! crypto: add Date fields for validTo and validFrom

* fix: move ArrayBuffer test patch

* src: fixup Error.stackTraceLimit during snapshot building

nodejs/node#55121

* fix: bad rebase

* chore: fixup amaro

* chore: address feedback from review

* src: revert filesystem::path changes

nodejs/node#55015

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
soobinrho pushed a commit to soobinrho/electron that referenced this pull request Jan 22, 2025
* chore: bump node in DEPS to v22.11.0

* src: move evp stuff to ncrypto

nodejs/node#54911

* crypto: add Date fields for validTo and validFrom

nodejs/node#54159

* module: fix discrepancy between .ts and .js

nodejs/node#54461

* esm: do not interpret "main" as a URL

nodejs/node#55003

* src: modernize likely/unlikely hints

nodejs/node#55155

* chore: update patch indices

* crypto: add validFromDate and validToDate fields to X509Certificate

nodejs/node#54159

* chore: fixup perfetto patch

* fix: clang warning in simdjson

* src: add receiver to fast api callback methods

nodejs/node#54408

* chore: fixup revert patch

* fixup! esm: do not interpret "main" as a URL

* fixup! crypto: add Date fields for validTo and validFrom

* fix: move ArrayBuffer test patch

* src: fixup Error.stackTraceLimit during snapshot building

nodejs/node#55121

* fix: bad rebase

* chore: fixup amaro

* chore: address feedback from review

* src: revert filesystem::path changes

nodejs/node#55015

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
codebytere added a commit to electron/electron that referenced this pull request Feb 3, 2025
codebytere added a commit to electron/electron that referenced this pull request Feb 10, 2025
codebytere added a commit to electron/electron that referenced this pull request Feb 10, 2025
codebytere added a commit to electron/electron that referenced this pull request Feb 14, 2025
codebytere added a commit to electron/electron that referenced this pull request Feb 14, 2025
* chore: bump node in DEPS to v22.13.0

* chore: bump node in DEPS to v22.13.1

* src: move evp stuff to ncrypto

nodejs/node#54911

* crypto: add Date fields for validTo and validFrom

nodejs/node#54159

* module: fix discrepancy between .ts and .js

nodejs/node#54461

* esm: do not interpret "main" as a URL

nodejs/node#55003

* src: modernize likely/unlikely hints

nodejs/node#55155

* chore: update patch indices

* crypto: add validFromDate and validToDate fields to X509Certificate

nodejs/node#54159

* chore: fixup perfetto patch

* fix: clang warning in simdjson

* src: add receiver to fast api callback methods

nodejs/node#54408

* chore: fixup revert patch

* fixup! esm: do not interpret "main" as a URL

* fixup! crypto: add Date fields for validTo and validFrom

* fix: move ArrayBuffer test patch

* src: fixup Error.stackTraceLimit during snapshot building

nodejs/node#55121

* fix: bad rebase

* chore: fixup amaro

* chore: address feedback from review

* src: revert filesystem::path changes

nodejs/node#55015

* chore: fixup GN build file

* nodejs/node#55529
* nodejs/node#55798
* nodejs/node#55530

* module: simplify --inspect-brk handling

nodejs/node#55679

* src: fix outdated js2c.cc references

nodejs/node#56133

* crypto: include openssl/rand.h explicitly

nodejs/node#55425

* build: use variable for crypto dep path

nodejs/node#55928

* crypto: fix RSA_PKCS1_PADDING error message

nodejs/node#55629

* build: use variable for simdutf path

nodejs/node#56196

* test,crypto: make crypto tests work with BoringSSL

nodejs/node#55491

* fix: suppress clang -Wdeprecated-declarations in libuv

libuv/libuv#4486

* deps: update libuv to 1.49.1

nodejs/node#55114

* test: make test-node-output-v8-warning more flexible

nodejs/node#55401

* [v22.x] Revert "v8: enable maglev on supported architectures"

nodejs/node#54384

* fix: potential WIN32_LEAN_AND_MEAN redefinition

c-ares/c-ares#869

* deps: update nghttp2 to 1.64.0

nodejs/node#55559

* src: provide workaround for container-overflow

nodejs/node#55591

* build: use variable for simdutf path

nodejs/node#56196

* chore: fixup patch indices

* fixup! module: simplify --inspect-brk handling

* lib: fix fs.readdir recursive async

nodejs/node#56041

* lib: avoid excluding symlinks in recursive fs.readdir with filetypes

nodejs/node#55714

This doesn't currently play well with ASAR - this should be fixed in a follow up

* test: disable CJS permission test for config.main

This has diverged as a result of our revert of
src,lb: reducing C++ calls of esm legacy main resolve

* fixup! lib: fix fs.readdir recursive async

* deps: update libuv to 1.49.1

nodejs/node#55114

---------

Co-authored-by: electron-roller[bot] <84116207+electron-roller[bot]@users.noreply.github.com>
Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. backported-to-v22.x PRs backported to the v22.x-staging branch. dont-land-on-v20.x PRs that should not land on the v20.x-staging branch and should not be released in v20.x. esm Issues and PRs related to the ECMAScript Modules implementation. module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run. strip-types Issues or PRs related to strip-types support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Possible discrepancy with --experimental-strip-types