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.mock() doesn't work well with NODE_PATH #3069

Closed
gaearon opened this issue Mar 5, 2017 · 5 comments
Closed

jest.mock() doesn't work well with NODE_PATH #3069

gaearon opened this issue Mar 5, 2017 · 5 comments

Comments

@gaearon
Copy link
Contributor

gaearon commented Mar 5, 2017

I am reporting what I consider to be a bug.
Originally reported in facebook/create-react-app#1271.

Some people use NODE_PATH to enable "absolute imports" in their codebase. For example, running NODE_PATH=. npm test makes every file and folder in the top-level directory available as require('name') rather than require('./name').

Current behavior

This approach works fine with Jest, but the mocking behavior is a bit confusing. jest.mock() only works with it correctly when jest.mock() matches the require. In other words, jest.mock('./a') will only work for files that happen to require('./a') but not for the ones that require('a'). Conversely, jest.mock('a') will only work for files that do require('a') but not require('./a'). Since both are possible when one sets NODE_PATH, it can be tricky to figure out what went wrong, and why jest.mock doesn't always work.

Expected behavior

When NODE_PATH is used, and a is a file or folder in that directory, both jest.mock('a') and jest.mock('./a') work the same way, and mock both require('a') and require('./a') calls.

Repro

git clone https://github.com/gaearon/jest-mock-issue-repro.git
cd jest-mock-issue-repro
npm i
npm test

See that the test fails. Open index.test.js and index.js.

Observe that jest.mock('./a', () => 'goodbye, '); only works if there's a require('./a') in index.js, and jest.mock('a', () => 'goodbye, '); only works if there's a require('a') in index.js.

I would expect that both work in both cases, and are considered equivalent, because they ultimately resolved to the same module.

What Does Node Do?

One could argue that it's intentional behavior because ./a and a are different paths, and Node is known to treat different paths as different modules, so Jest should do the same.

However I don't think this is the case with NODE_PATH.
With code like this:

require('./a');
require('./A');

Node will indeed execute the module twice.

However, with NODE_PATH=. and calls like this:

require('a');
require('./a');

Node will still execute the module just once. This means that the module ID should be based on path after resolving, and I argue that jest.mock() should also treat them as the same module.

@abdulhannanali
Copy link
Contributor

@DmitriiAbramov Interesting stuff would love to take a look into it and help resolve this bug. This seems to be an issue probably with jest-resolve or jest-mock. However, I'll definitely give a shot at resolving this.

@aaronabramov
Copy link
Contributor

cc @wanderley who's been working on jest-resolve

@johann-sonntagbauer
Copy link

johann-sonntagbauer commented Apr 10, 2017

@abdulhannanali I face the same problem. Had you any success so far? Maybe we could join forces.

What I found out so far:
The problem is caused by module resolution.
jest runtime line 274 called with absolute and relative modules resulted in following states:

modulePath = this._resolveModule (from, moduleName)

componentRegistry Module resolved relative

from: C:\Projects\client\src\core\component\hoc\configurable.js
moduleName: ./../componentRegistry
modulePath: C:\Projects\client\src\core\component\componentRegistry.js

componentRegistry Module resolved through NODE_PATH directory

from: C:\Projects\client\src\core\component\hoc\__tests__\configurable.test.js
moduleName: core/component/componentRegistry
modulePath: src\core\component\componentRegistry.js

Because the modulePath is different, the map will not recognize that the modules are the same and run the module again.

As @gaearon already mentioned should the module ID be based on the absolute path. But I am not sure how that influences the resolution mechanism with haste packages.

@gaearon
Copy link
Contributor Author

gaearon commented May 25, 2017

I can confirm #3616 fixed this.

@gaearon gaearon closed this as completed May 25, 2017
@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants