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.

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

feat: support files other than js to be ESM #10823

Merged
merged 4 commits into from
Dec 4, 2020

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Nov 14, 2020

Summary

Fixes #9860. See that issue for some discussion.

We could have an option somewhere for transformers to decide, but then we'd have have some forking logic in jest-runtime. Just having a top level option seems easier to reason about to me. Open to being convinced otherwise, tho! 👍

Test plan

E2E test added, plus unit tests for the config verification.

@thymikee
Copy link
Collaborator

If we're running in ESM mode, shouldn't we treat all moduleFileExtensions (maybe except json) as modules then? I just wonder if we could get away without adding a new config option.

@SimenB
Copy link
Member Author

SimenB commented Nov 14, 2020

How would ESM mode be "triggered"? Also, it's perfectly fine to use import on CJS from ESM or import() on ESM from CJS so being in "one mode" is not really possible unless your entire dependency tree is uniform.

@SimenB SimenB force-pushed the support-non-js-esm branch 3 times, most recently from 1dec5a1 to 0a103ee Compare November 15, 2020 14:35
@SimenB
Copy link
Member Author

SimenB commented Nov 15, 2020

I'm thinking we can land this so people can test this, and if we figure out a better approach in the future we can revert

@ahnpnl
Copy link
Contributor

ahnpnl commented Nov 15, 2020

I think custom transformer guidance also needs updating ?

@SimenB
Copy link
Member Author

SimenB commented Nov 16, 2020

I think custom transformer guidance also needs updating ?

Done in #10834 - this PR doesn't change that fact (i.e. use supportsStaticESM to check if import should be used - this PR just allows the user to pass config which makes sure that flag is set to true instead of false for non-JS)

@SimenB SimenB force-pushed the support-non-js-esm branch from 0d41121 to e8f3cc4 Compare November 26, 2020 18:12
@SimenB SimenB force-pushed the support-non-js-esm branch from 44e87b6 to d4600f5 Compare December 4, 2020 12:09
@SimenB SimenB force-pushed the support-non-js-esm branch from 1892952 to 7ffb156 Compare December 4, 2020 12:12
@SimenB SimenB merged commit 64de4d7 into jestjs:master Dec 4, 2020
@SimenB SimenB deleted the support-non-js-esm branch December 4, 2020 13:03
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SyntaxError: Cannot use import statement outside a module, with TypeScript and ES Modules
4 participants