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: add support for dynamic imports in CJS #10620

Merged
merged 2 commits into from Oct 11, 2020

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Oct 11, 2020

Summary

Since we no longer use compileFunction, we can now implement this.

Test plan

Test added

@SimenB SimenB merged commit 5c221d8 into jestjs:master Oct 11, 2020
@SimenB SimenB deleted the dynamic-import-cjs branch October 11, 2020 15:04
@nicolo-ribaudo
Copy link
Contributor

nicolo-ribaudo commented Oct 30, 2020

Should this work on Node.js 14.15.0 with Jest?

I have a CJS function like this one:

"use strict";

exports.default = import_;

function import_(filepath) {
  let res = import(filepath);
  throw new Error("Ok 2");
  return res;
}

When I call it with a valid filepath, Jest exits with a non-zero exit code without reporting any failure (I would expect it to throw "Ok 2").

I thus tried adding require("fs").writeFileSync("/tmp/linkModules-called", "ok"); as the first line of the linkModules function (which in this PR is directly used by importModuleDynamically) to check if it's called and it seems that it is never called 🤔

EDIT: It's a segmentation fault (core dumped), I'll try creating a smaller example and open a new issue.

@SimenB
Copy link
Member Author

SimenB commented Oct 30, 2020

Should this work on Node.js 14.15.0 with Jest?

Definitely, as can be seen from the test here, this runs successfully on node@"^12.16.0 || >=13.7.0"

EDIT: It's a segmentation fault (core dumped), I'll try creating a smaller example and open a new issue.

That doesn't sound good 🙈 🙉 Ping me when/if you're able to put together a repository. If it's in Babel I'm happy to take a look at some branch

@pleunv
Copy link

pleunv commented Nov 4, 2020

Does this mean that babel-plugin-dynamic-import-node wouldn't be necessary anymore, or am I misunderstanding this PR?

@SimenB
Copy link
Member Author

SimenB commented Nov 4, 2020

correct. If you use preset-env it will be disabled by default, but if you explicitly add it you can remove it

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

None yet

4 participants