Skip to content

Conversation

@bmeck
Copy link
Member

BREAKING (kind of a bugfix? unclear)

This ensures files with unknown extensions like extension.unknown are not
loaded as CJS/ESM when imported as a main entry point and makes
sure that those files would maintain the same format even if loaded
after the main entrypoint.

Added tests for importing missing extension and unknown extension after entrypoint along the way.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • tests and/or benchmarks are included
  • documentation is changed or added
  • commit message follows commit guidelines

This ensures files with unknown extensions like foo.bar are not loaded as CJS/ESM when imported as a main entry point and makes sure that those files would maintain the same format even if loaded after the main entrypoint.
@bmeckbmeck added the esm Issues and PRs related to the ECMAScript Modules implementation. label Dec 18, 2019
@bmeck
Copy link
MemberAuthor

CC: @nodejs/modules-active-members

Copy link
Contributor

@guybedfordguybedford left a comment

Choose a reason for hiding this comment

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

One of the major features of the ES module resolver is being able to support new file extensions in future, which is reliant on the fact that we always throw for unknown file extensions.

Giving special treatment to isMain was how we did this while ensuring compatibility with existing bins.

Happy to flesh this out further, but I don't think we should lose that extension property for the ESM resolver.

@bmeck
Copy link
MemberAuthor

It seems like there might be something more that needs to be done for node ./entry-point.wasm to handle WASI in addition to the changes here. Asking around about how main() and import are setup for WASI and it seems that entry points have some special behavior. We should ensure that is handled but likely would be a follow on PR instead of on top this one.

@nodejs-github-bot
Copy link
Collaborator

@guybedford
Copy link
Contributor

It seems like there might be something more that needs to be done for node ./entry-point.wasm to handle WASI in addition to the changes here. Asking around about how main() and import are setup for WASI and it seems that entry points have some special behavior. We should ensure that is handled but likely would be a follow on PR instead of on top this one.

We are likely still missing the proper work on how Web Assembly integrates into the module system. There might be Web Assembly headers in future that specify the top-level resolution (like package.json). There is also some work to allow Web Assembly start functions to themselves run instantiation (removing the need for JS to be primary entry). Node.js should definitely track whatever happens here as it stabilizes.

@nodejs-github-bot
Copy link
Collaborator

bmeck pushed a commit that referenced this pull request Dec 31, 2019
This ensures files with unknown extensions like foo.bar are not loaded as CJS/ESM when imported as a main entry point and makes sure that those files would maintain the same format even if loaded after the main entrypoint. PR-URL: #31021 Reviewed-By: Guy Bedford <[email protected]>
@bmeck
Copy link
MemberAuthor

Landed in baa3621

@bmeckbmeck closed this Dec 31, 2019
BridgeAR pushed a commit that referenced this pull request Jan 3, 2020
This ensures files with unknown extensions like foo.bar are not loaded as CJS/ESM when imported as a main entry point and makes sure that those files would maintain the same format even if loaded after the main entrypoint. PR-URL: #31021 Reviewed-By: Guy Bedford <[email protected]>
@BridgeARBridgeAR mentioned this pull request Jan 7, 2020
MylesBorins pushed a commit that referenced this pull request Jan 12, 2020
This ensures files with unknown extensions like foo.bar are not loaded as CJS/ESM when imported as a main entry point and makes sure that those files would maintain the same format even if loaded after the main entrypoint. PR-URL: #31021 Reviewed-By: Guy Bedford <[email protected]>
@targostargos mentioned this pull request Jan 15, 2020
GeoffreyBooth added a commit to GeoffreyBooth/node that referenced this pull request Jan 19, 2020
GeoffreyBooth added a commit to GeoffreyBooth/node that referenced this pull request Jan 20, 2020
GeoffreyBooth added a commit that referenced this pull request Jan 23, 2020
reverses baa3621 PR-URL: #31415 Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Bradley Farias <[email protected]> Reviewed-By: Rich Trott <[email protected]>
BethGriggs pushed a commit that referenced this pull request Feb 6, 2020
This ensures files with unknown extensions like foo.bar are not loaded as CJS/ESM when imported as a main entry point and makes sure that those files would maintain the same format even if loaded after the main entrypoint. PR-URL: #31021 Reviewed-By: Guy Bedford <[email protected]>
@MylesBorinsMylesBorins mentioned this pull request Feb 8, 2020
codebytere pushed a commit that referenced this pull request Feb 17, 2020
reverses baa3621 PR-URL: #31415 Reviewed-By: Guy Bedford <[email protected]> Reviewed-By: Gus Caplan <[email protected]> Reviewed-By: Bradley Farias <[email protected]> Reviewed-By: Rich Trott <[email protected]>
@codebyterecodebytere mentioned this pull request Feb 17, 2020
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

esmIssues and PRs related to the ECMAScript Modules implementation.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants

@bmeck@nodejs-github-bot@guybedford@bfarias-godaddy