-
-
Notifications
You must be signed in to change notification settings - Fork 22
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
Add support for setupFiles
and snapshotSerializers
#4
Conversation
|
a09841b
to
dc3ea69
Compare
src/worker-runner.js
Outdated
if (setup && setup.default) { | ||
await setup.default; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@SimenB, should setupFiles
work like this?
We exposed setup
function before, https://github.com/prettier/prettier/blob/3cbee742a792413211eea1e4fe2d0dad26ff37ea/tests/config/jest-light-runner/src/worker-runner.js#L30-L36
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking at the Jest source code (https://github.com/facebook/jest/blob/main/packages/jest-repl/src/cli/runtime-cli.ts#L112-L123=), we should do something like this:
const { default: setup } = await import(pathToFileURL(setupFile));
if (typeof setup === "function") await setup();
(technically Jest only calls setup()
if it comes from a CJS file, but it's easier to always call it than to check the file type).
In Prettier's setup file, you could either:
- rewrite it to CJS, use dynamic imports in the
setup
function instead of top-level import declarations, and domodule.exports = setup
(this also works with Jest's default runner); - export a function named
then
rather thansetup
, becauseawait import(...)
will automatically callthen
and await it since{ then: function }
is a promise-like object (this might also work with Jest's default runner); - Use
export default setup
(and notexport default setup()
), so that it will be awaited because it's a function (this will only work with this runner, since Jest only calls the function for CJS files).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah. we would like to call it for ESM as well, but there's no (supported) way of getting the exported result with vm
APIs. So our plan there is to tell people to use TLA for ESM and exporting a function from CJS.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! 😄
Could you also add them in the README
to the "supported features" section?
src/worker-runner.js
Outdated
let serializer = await import(pathToFileURL(snapshotSerializer)); | ||
|
||
if (serializer && serializer.default) { | ||
serializer = serializer.default; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be enough, since the serializer is always stored on module.exports
and thus becomes the default export:
const { default: serializer } = await import(pathToFileURL(snapshotSerializer));
Also, probably we should add them in reverse order: https://github.com/facebook/jest/blob/a20bd2c31e126fc998c2407cfc6c1ecf39ead709/packages/jest-circus/src/legacy-code-todo-rewrite/jestAdapterInit.ts#L149-L152
All review points addressed. |
Thanks! I'll add a CI job that tests this runner both with Babel's and Prettier's test suites, then I'll release a new version. |
Thank you! |
FYI: |
I'm going to work on setting up Prettier's test. |
Prettier added these when using this module.
https://github.com/prettier/prettier/tree/next/tests/config/jest-light-runner