jestjs / jest

Delightful JavaScript Testing.
https://jestjs.io
MIT License
44.08k stars 6.44k forks source link

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

Open gaearon opened 6 years ago

gaearon commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

gaearon commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

Oh man. This is why I hate configuration shortcuts 😄

SimenB commented 6 years ago

@cpojer @aaronabramov thoughts?

gaearon commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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

SimenB commented 6 years ago

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

octogonz commented 6 years ago

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 commented 5 years ago

@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 commented 4 years ago

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 commented 4 years ago

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[bot] commented 2 years ago

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[bot] commented 2 years ago

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 commented 2 years ago

We should still do this, probably

weronikadominiak commented 7 months ago

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

SimenB commented 7 months ago

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