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

Jest should resolve its dependencies from its own location rather than rootDir #5913

Open
gaearon opened this issue Apr 3, 2018 · 21 comments

Comments

@gaearon
Copy link
Contributor

gaearon commented Apr 3, 2018

Create React App users bumped into this issue recently. A third party package started depending on jest-validate, and that caused npm to hoist jest-environment-node to the top of the app tree. Unfortunately, Jest resolves environment package from the project root instead of from its own Node module location so as a result, it loads the wrong (hoisted) jest-environment-node.

I think the jest package should resolve any own dependencies from its own location (just like Node resolution works) rather than from the project root folder.

@gaearon
Copy link
Contributor Author

gaearon commented Apr 3, 2018

I guess the problem is that an environment might need to be searched for in the project folder if the project explicitly specifies it. Perhaps it's worth including the project folder if environment exists in package.json?

@SimenB
Copy link
Member

SimenB commented Apr 3, 2018

Nice detective work, this is indeed a subtle and annoying bug.

I think it makes sense to special case (jest-environment-)node and (jest-environment-)jsdom and skip the Resolver.findNodeModule stuff for them.

@johannes-scharlach
Copy link

Rather than special module resolution logic, maybe relying on explicit versions of peer packages might be the best way to go?

Declaring all dependencies and strictly sticking to semver should also work, but semver is a lot harder to follow in monorepos according to my own experience. I've also had a similar bug with jest recently - since it's a dev dependency I wouldn't mind the lack of deduplication due to exact module resolution.

@SimenB
Copy link
Member

SimenB commented Apr 3, 2018

I think we want to avoid peerDependencies to keep the "batteries included" feeling.

@gaearon
Copy link
Contributor Author

gaearon commented Apr 3, 2018

I think this:

I think it makes sense to special case (jest-environment-)node and (jest-environment-)jsdom and skip the Resolver.findNodeModule stuff for them.

could be confusing for people who specifically want to pin their versions (e.g. to avoid a bug).

It would make more sense to me if Jest only searched the app folder when the app's package.json declares those as explicit deps.

@SimenB
Copy link
Member

SimenB commented Apr 3, 2018

We can do that. Should we only check for jest-environment-* in package.json? E.g. jest-environment-jsdom can be shorthanded in config as jsdom, but we don't wanna load jsdom itself if it exists in package.json. But there is no hard requirement of environments having the jest-environment- prefix, only a convention

@gaearon
Copy link
Contributor Author

gaearon commented Apr 3, 2018

Oh man. This is why I hate configuration shortcuts 😄

@SimenB
Copy link
Member

SimenB commented Apr 3, 2018

@cpojer @aaronabramov thoughts?

@gaearon
Copy link
Contributor Author

gaearon commented Apr 3, 2018

I think I expect that if I have jest-environment-jsdom in my dependencies then no matter whether I specify --env=jsdom or --env=jest-environment-jsdom, it will use my local version.

If I don't specify it in my dependencies, I expect Jest to use its local version by resolving it from jest-config.

Finally, if I specify some other environment (not jsdom or node), I expect Jest to always resolve it from my app, and not even attempt to resolve it from jest-config.

In other words, the only case in which I want Jest to resolve env from its own jest-config location is if I didn't specify the full name of the corresponding env in my package.json as a dependency. Neither node nor jsdom are considered valid third party env names.

@SimenB
Copy link
Member

SimenB commented Apr 4, 2018

So if the env is one of the two shipped with jest, look for them in root deps, if there use them, otherwise resolve from root? Unknown envs are resolved from root no matter what. Seems reasonable to me :)

Should we just care about dependencies and devDependencies? What about peerDependencies? Or yarn's resolutions?

@gaearon
Copy link
Contributor Author

gaearon commented Apr 4, 2018

Any explicit mention seems like fair game to try resolving from app root (IMO).

@SimenB
Copy link
Member

SimenB commented Apr 29, 2018

@cpojer @mjesun thoughts on this? It would be awesome to resolve this before 23 drop

@octogonz
Copy link

I proposed a different solution in the comments for PR #6145. The fundamental issue here seems to be that the Jest configuration specifies the test environment as a naive package name, but the package version is determined by brittle heuristics that vary between package managers. It completely falls apart for my situation where we need two different versions of Jest to be installed side-by-side. Tuning the brittle heuristics seems like a step in the wrong direction.

The testEnvironment config allow a concrete specification such as module path or an already imported module object (for runtime configs). This could easily be introduced without breaking backwards compatibility.

Updating jest-runner to have a peer dependency or regular dependency on jest-environment-jsdom is a quick solution (and mandatory in theory since jsdom is loaded by default), but it may break some consumers in minor ways.

@SimenB
Copy link
Member

SimenB commented Feb 1, 2019

@gaearon one thing you can do to fix this is give Jest absolute paths (e.g. doing require.resolve on testEnvironment rather than leaving resolution up to Jest). Jest uses the absolute path for the default now (as of Jest 24), but if you pass e.g. node or jsdom you might still get the wrong resolved version.

@ChromeQ
Copy link

ChromeQ commented May 4, 2020

Can this issue be looked into again?
I've run into an issue that the workaround in CRA is forcing a cli argument for testEnvironment which is always overriding the ones I supply in the jest config under projects.
I want to use different environments for front end and backend code, which should be possible with jest config projects.

@demeralde
Copy link

demeralde commented May 6, 2020

Running into this issue and not sure how to solve it. Anyone know what to do?

I've installed redux-actions, which has its own reduce-reducers dependency. Now after installing reduce-reducers for my project, it's causing errors (because redux-actions expects reduce-reducers to be a different version).

This works fine in my app and is therefore set up correctly, it's just that the Jest tests are now failing because Jest is loading the wrong version.

For now I've just installed the version redux-actions expects as a workaround, but I think Jest should handle module resolution the same way Node does.

@github-actions
Copy link

This issue is stale because it has been open for 1 year with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions github-actions bot added the Stale label Feb 25, 2022
@github-actions
Copy link

This issue was closed because it has been stalled for 7 days with no activity. Please open a new issue if the issue is still relevant, linking to this one.

@SimenB
Copy link
Member

SimenB commented Apr 26, 2022

We should still do this, probably

@SimenB SimenB reopened this Apr 26, 2022
@SimenB SimenB added Pinned and removed Stale labels Apr 26, 2022
@SimenB SimenB added this to the High priority future milestone Apr 26, 2022
@weronikadominiak
Copy link

Hi, is there any chance this will be looked at?

@SimenB
Copy link
Member

SimenB commented Jan 29, 2024

PR very much welcome 🙂 I don't have any plans to work on this myself, but happy to review PRs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants