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.

Sign up for GitHub

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

Jump to bottom

feat: pass conditions when resolving modules #11859

Merged
merged 11 commits into from Sep 13, 2021
Merged

feat: pass conditions when resolving modules #11859

Changes from 1 commit
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
260eaaa
feat: pass `conditions` when resolving modules
SimenB Sep 9, 2021
839edc0
ci
SimenB Sep 9, 2021
0f29eb5
do not pass require condition for esm
SimenB Sep 10, 2021
127071a
add resolve options to cache key
SimenB Sep 10, 2021
9afb6ef
snaps again
SimenB Sep 10, 2021
30b8beb
resolve comment
SimenB Sep 10, 2021
980ff26
oops
SimenB Sep 10, 2021
03abc32
lint
SimenB Sep 10, 2021
b7e2b20
docs
SimenB Sep 10, 2021
f3be5cd
changelog
SimenB Sep 10, 2021
8a7f59a
prettier
SimenB Sep 10, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
10 changes: 7 additions & 3 deletions packages/jest-runtime/src/index.ts
View file
Open in desktop
Expand Up @@ -173,7 +173,7 @@ const supportsNodeColonModulePrefixInImport = (() => {
return stdout === 'true';
})();

// consider "node" condition as well - maybe switching on `global.window` in the test env?
// consider `node` & `browser` condition as well - maybe switching on `global.window` in the test env?
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe they could be enabled by jest-environment-* packages? I don't know if the internal architecture makes it impossible, but from my (external, user) point of view it's what makes more sense.

Copy link
Member Author

@SimenB SimenB Sep 10, 2021 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely a possibility! Main worry is browser meaning either ESM or CJS, but I guess we can add the option and just not use it ourselves for now

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll open up a separate PR with that after landing this 🙂

const esmConditions = ['import', 'default'];
const cjsConditions = ['require', 'default'];

Expand Down Expand Up @@ -299,8 +299,12 @@ export default class Runtime {
this._virtualMocks,
filePath,
undefined,
// is this correct?
{conditions: cjsConditions},
// shouldn't really matter, but in theory this will make sure the caching is correct
{
conditions: this.unstable_shouldLoadAsEsm(filePath)
? esmConditions
: cjsConditions,
},
);
this._transitiveShouldMock.set(moduleID, false);
}
Expand Down