Skip to content

Conversation

@bmeck
Copy link
Member

@bmeck bmeck commented Dec 18, 2019

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.
@bmeck bmeck added the esm Issues and PRs related to the ECMAScript Modules implementation. label Dec 18, 2019
@bmeck bmeck requested review from guybedford and hybrist December 18, 2019 16:04
@bmeck
Copy link
Member Author

bmeck commented Dec 18, 2019

CC: @nodejs/modules-active-members

Copy link
Contributor

@guybedford guybedford 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
Member Author

bmeck commented Dec 19, 2019

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

bmeck commented Dec 31, 2019

Landed in baa3621

@bmeck bmeck 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]>
@BridgeAR BridgeAR 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]>
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]>
@MylesBorins MylesBorins 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]>
@codebytere codebytere mentioned this pull request Feb 17, 2020
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants