Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork6.6k
feat(snapshot): support snapshotResolver and snapshotSerializers written in ESM#12014
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
base:main
Are you sure you want to change the base?
Uh oh!
There was an error while loading.Please reload this page.
Conversation
f8aa171 to2cab732Compare| it('defining tests and hooks asynchronously throws',()=>{ | ||
| constresult=runJest('circus-declaration-errors',[ | ||
| it('defining tests and hooks asynchronously throws',async()=>{ | ||
| constresult=awaitrunJest('circus-declaration-errors',[ |
chentsulinOct 30, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
ThisrunJest function actually doesn't return a promise, so we should find a way to test this kind of stuff.
This test got failed after I putruntime.requireModule into a async function.
From:
constesm=runtime.unstable_shouldLoadAsEsm(testPath);if(esm){awaitruntime.unstable_importModule(testPath);}else{runtime.requireModule(testPath);}
To:
constlocalRequire=async<T=unknown>(path:string):Promise<T>=>{constesm=runtime.unstable_shouldLoadAsEsm(path);if(esm){returnruntime.unstable_importModule(path)asany;}else{returnruntime.requireModule(path);}};awaitlocalRequire(testPath);
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.
runJest spawnsjest in a shell, so it should not be affected by any internal changes in Jest
933e7ca tob961505Compared19c3af toddb4eebCompare| constevaluatedModule=awaitthis.linkAndEvaluateModule(module); | ||
| returnevaluatedModuleinstanceofSourceTextModule | ||
| ?evaluatedModule.namespace.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.
I have no clue what to do here, but I found this method sometimes returnsvm.SourceTextModule as result.
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.
There is no "result" of executing a module. Do we need access to evaluated module? We haven't needed that before... We might need anunstable_importInternalModule which essentially does what you do here (wait forevaluatedModule.status === 'evaluated', then fetch its default export).
Essentially faking animport() call
chentsulinNov 3, 2021 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
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.
If i didn't get it wrong, the result is used by thislocalRequire call to support ESM resolvers:
constcustom:SnapshotResolver=awaitlocalRequire(snapshotResolverPath,true,);
| ).default; | ||
| constcustom:SnapshotResolver=awaitlocalRequire( | ||
| snapshotResolverPath, | ||
| true, |
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.
Add a boolean parameter to localRequire to supportinteropRequireDefault here.
d65360d tod8033b6Compared8033b6 tof7eb057CompareMaxim-Mazurok commentedSep 28, 2022
@chentsulin I wonder if this could be finalized or if it is waiting for a re-review from@SimenB ?... We need this here:https://github.com/Maxim-Mazurok/google-api-typings-generator/blob/062854a2021be2dd244855261a73f5172bf597c2/custom-resolver.cjs for the use-case of using one snapshot file per test. Thank you! |
SimenB commentedSep 28, 2022
Main issue is that we load the resolverwithin the tests (#12014 (comment)), which means it's pretty much blocked by full ESM support |
Maxim-Mazurok commentedSep 28, 2022 • edited
Loading Uh oh!
There was an error while loading.Please reload this page.
edited
Uh oh!
There was an error while loading.Please reload this page.
Ah, I see, thank you for the update, weare re-considering our approach now. Cheers! |
SimenB commentedSep 28, 2022
It might make sense to not load it within, but that means people who have written it in TS or something that requires transpilation will fail. So not sure which tradeoff is the best here |
Anutrix commentedSep 21, 2023
It's been a year. Any updates? |
This PR is stale because it has been open 1 year with no activity. Remove stale label or comment or this will be closed in 30 days. |
Anutrix commentedSep 22, 2024
It's been another year. Any updates? |
cpojer commentedMay 22, 2025
Could this PR be rebased so we can merge it for Jest 30? |
SimenB commentedMay 27, 2025
Following up my old comment - I think we can just load the resolver outside of the tests. That gives ESM support, and if people wanna write it in TS they can usehttps://nodejs.org/en/learn/typescript/run-natively |
Uh oh!
There was an error while loading.Please reload this page.
Summary
Part of#11167.
See the discussion on the
snapshotResolverandsnapshotSerializersconfig:#11167 (comment)#11167 (comment)Test plan
Integration test added.