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

Allow mocking require.resolve #9543

Open
nicolo-ribaudo opened this issue Feb 9, 2020 · 26 comments
Open

Allow mocking require.resolve #9543

nicolo-ribaudo opened this issue Feb 9, 2020 · 26 comments

Comments

@nicolo-ribaudo
Copy link
Contributor

🚀 Feature Proposal

The require.resolve function is re-created for every file: this is needed so that it starts resolving from the correct location.

However, this makes it impossible to mock it: require.resolve = jest.fn() doesn't work.
I propose adding an API similar to jest.mock specifically for require.resolve's behavior.

Motivation

We have different tests in @babel/core that test plugins/presets aliasing (ref). For example, we test that when someone uses @babel/env in their config Babel tries to resolve @babel/preset-env.

We currently rely on "fake" node_modules folders, however:

  1. We don't want to test node's resolution algorithm, we should only test what we are asking node to resolve.
  2. Using real FS just to check plugins/presets aliasing makes our tests slower.
  3. This approach doesn't work with Yarn PnP, because it changes the resolution behavior (and it shouldn't affect our tests).

Example

Maybe something like this?

jest.mockResolution("@babel/env", __dirname + "/fake/@babel/preset-env");

// test

jest.unmockResolution("@babel/env");

Pitch

Mocks are already handled by the Jest core platform 😁

@SimenB
Copy link
Member

SimenB commented Feb 9, 2020

Oooh, I like it! It would essentially be jest.mock but only instead of providing the inline implementation, you provide the path it should resolve to? Would allow people to easily reuse mock modules without putting a file in __mocks__/ directories as well.

Not a huge fan of needing to replicate all of .mock, doMock, unmock and dontMock though... Would overloading be confusing to people? So if a function is provided it's a inline mock, if it's a string, it's the resolution that's mocked? I generally hate overloaded APIs, but in this case I think it might make sense?

@nicolo-ribaudo
Copy link
Contributor Author

Somehow I didn't realize that my proposal could be directly related to jest.mock 😂 I think that your proposed overload makes sense, and might even be more intuitive than having to use a parallel API.

Do you think that require.resolve should check if the mocked path exists? I'd say no, but then we would need something to mock a file as non-existing.

@SimenB
Copy link
Member

SimenB commented Feb 10, 2020

We have jest.mock('thing', factory, {virtual: true}) if you don't want the thing you're mocking to exist. I think that should work with require.resolve as well, no?

/cc @jeysal @thymikee @cpojer thoughts on this one?

@nicolo-ribaudo
Copy link
Contributor Author

Oh, I didn't know about virtual.

If no one from the team has concerns with this feature request, I could start working on an implementation next week.

@cpojer
Copy link
Member

cpojer commented Feb 10, 2020

I think virtual mocks should cover your use case. Let's try that first before adding more APIs to Jest. I do not think that mocking require.resolve is desirable, there are too many ways to shoot yourself in the foot :D

@nicolo-ribaudo
Copy link
Contributor Author

Since Babel internally directly calls require.resolve, I don't see how virtual mocks could cover my use case 🤔

@cpojer
Copy link
Member

cpojer commented Feb 10, 2020

You should be able to create virtual mocks to ensure that require.resolve doesn't throw. If it throws, you know Babel did something unexpected.

You can also require the module after calling require.resolve and then verify that the mock module function was called etc.

@nicolo-ribaudo
Copy link
Contributor Author

nicolo-ribaudo commented Feb 10, 2020

It doesn't seem to work:

describe.only("virtual mocks", function() {
  it("finds preset", () => {
    try {
      const preset = () => ({});

      // also tested with "../../../node_modules/babel-preset-test-1234"
      jest.doMock("babel-preset-test-1234", () => preset, { virtual: true });

      expect(() =>
        babel.transformSync("code;", {
          presets: ["babel-preset-test-1234"],
          configFile: false,
        }),
      ).not.toThrow();
    } finally {
      jest.dontMock("babel-preset-test-1234");
    }
  });
});

reports

  ● virtual mocks › finds preset

    expect(received).not.toThrow()

    Error name:    "Error"
    Error message: "Cannot resolve module 'babel-preset-test-1234' from paths ['/home/nicolo/Documenti/dev/babel/babel'] from /home/nicolo/Documenti/dev/babel/babel/packages/babel-core/lib/config/files/plugins.js"

          89 | 
          90 |   try {
        > 91 |     return require.resolve(standardizedName, {
             |                    ^
          92 |       paths: [dirname]
          93 |     });
          94 |   } catch (e) {

          at Runtime._requireResolve (node_modules/jest-runtime/build/index.js:892:13)
          at resolveStandardizedName (packages/babel-core/lib/config/files/plugins.js:91:20)
          at resolvePreset (packages/babel-core/lib/config/files/plugins.js:48:10)
          at loadPreset (packages/babel-core/lib/config/files/plugins.js:67:20)
          at createDescriptor (packages/babel-core/lib/config/config-descriptors.js:154:9)
          at packages/babel-core/lib/config/config-descriptors.js:109:50
              at Array.map (<anonymous>)
          at createDescriptors (packages/babel-core/lib/config/config-descriptors.js:109:29)
          at createPresetDescriptors (packages/babel-core/lib/config/config-descriptors.js:101:10)

      14 |           configFile: false,
      15 |         }),
    > 16 |       ).not.toThrow();
         |             ^
      17 |     } finally {
      18 |       jest.dontMock("babel-preset-test-1234");
      19 |     }

      at Object.<anonymous> (packages/babel-core/test/resolution.js:16:13)

Test Suites: 1 failed, 1 total
Tests:       1 failed, 40 skipped, 41 total
Snapshots:   0 total
Time:        0.634s

It doesn't work because the jest replacement for require checks for mocks, but the replacement for require.resolve doesn't.

Also, I don't know what require.resolve would be currently expected to return with mocks, because it must return a string which we aren't providing to jest.mock.

@cpojer
Copy link
Member

cpojer commented Feb 10, 2020

Thanks for trying it out and checking. That's a good observation that I missed. Now the question is: should require.resolve consider mocks and give a fake temporary file-path or should we allow mocking require.resolve?

@nicolo-ribaudo
Copy link
Contributor Author

nicolo-ribaudo commented Feb 10, 2020

If require.resolve("foo") returned something like "/tmp/jest-virtual-mocks/node_modules/foo" if an only if foo is a mocked module, then it would probably cover our use case. However, Jest should make calls to require("/tmp/jest-virtual-mocks/node_modules/foo") return the mocked implementation otherwise require(require.resolve("foo")) won't work.

I still think that explicitly specifying to Jest what to return when require.resolveing a mocked module would be better. Maybe something like this?

jest.mock("foo", () => {}, { virtual: true, resolve: "/tmp/node_modules/foo" });

Also, I realized that my original proposal doesn't cover a use case we would need in half of our resolution-related tests: checking that the correct error message is thrown when require.resolveing a module that doesn't exist (ref). This currently already works, but it checks every folder up to the root of the real FS because it uses the "real" algorithm.

It probably would need a separate option, which makes a mock throw whenever require or require.resolve are called:

jest.mock("foo", () => {}, { virtual: true, missing: true });

@cpojer
Copy link
Member

cpojer commented Feb 10, 2020

I'm a bit hesitant of adding new APIs to Jest because you are running into a very limited use case that not many people are encountering. I think I would prefer simply making it so you can overwrite require.resolve, then you can do whatever you like with it.

@SimenB
Copy link
Member

SimenB commented Feb 10, 2020

What I like about expanding jests API is that you can have reusable mocks (possibly distributed through npm) without __mocks__ directory. Also, it's weird that jest.mock affects require but not require.resolve.

@nicolo-ribaudo
Copy link
Contributor Author

nicolo-ribaudo commented Feb 10, 2020

I'm a bit hesitant of adding new APIs to Jest because you are running into a very limited use case that not many people are encountering.

I agree with this sentiment. However, I don't think that mine is a limited use case:

However, I didn't find anyone else asking how to mark a mocked module as not existing. Let's keep the discussion focused only on the original proposal.
Also, it could probably be worked around with @SimenB's original proposal (#9543 (comment)).

What I like about expanding jests API is that you can have reusable mocks (possibly distributed through npm) without __mocks__ directory.

This is something I personally don't need, but I definitely see how it would be useful for the rest of the community!

Also, it's weird that jest.mock affects require but not require.resolve.

💯 The require and require.resolve node APIs are designed to be symmetrical (I think that require itself uses require.resolve internally). Currently, this simmetry is broken when using jest.mock.

@SimenB
Copy link
Member

SimenB commented Feb 11, 2020

I'm down with adding a resolve option next to virtual. I think almost all of the code changes needed in jest-runtime will be the exact same regardless of where the signal comes from (a separate option, string as factory or an entirely new API, or something else we haven't thought of), and the bulk of the work will remain the same no matter what API we land on.

One challenge with overloading is that babel-plugin-jest does some checking on the type provided. You're probably the most qualified person around to make the changes to the Babel plugin, though 😛
https://github.com/facebook/jest/blob/47b8aaec0a6bb80b96f7641d68c76587d86abe23/packages/babel-plugin-jest-hoist/src/index.ts#L97-L102
Should probably be noted though that we might need to validate resolve in a similar way, since hoisting will make it so that e.g. jest.mock('./thing', () => {}, {resolve: findSomeUrl()}) can behave differently. Right now we force you to have everything in scope of the mock factory, be a global or be named mock*Something*. Something like it probably makes sense for whatever we end up adding to allow mocking require.resolve as well

@nicolo-ribaudo
Copy link
Contributor Author

Sure, I will prepare a draft PR in the next days!
I think that I will try to implement both the resolve option and the jest.fn(name, path) overload, because I'm not sure yet about which approach could be more valuable, or would add more complexity to the code.
We can then decide which is better by looking at the real implementations/tests 😉

@nicolo-ribaudo
Copy link
Contributor Author

Hey, sorry all for the super long time without any response.

I tried implementing it (I wanted to implement both the proposals, to check which one was better), but didn't manage to figure out how the require/resolve code works.

If anyone wants to implement it it would be highly appreciated, but I can still work on updating the Babel plugin if needed.

@TheHumbleJester
Copy link

TheHumbleJester commented Apr 20, 2020

Hey guys !
I know that's not exactly what you're looking for, but here's my workaround to be able to "mock" require.resolve.
There's an interesting feature in jest allowing you to define your own module resolver:
https://jestjs.io/docs/en/configuration#resolver-string

Having that in mind, here is, basically, what I've done:

  • I created my own resolver like so:
const glob = require('glob');

let mapping = {};

// Looks for "module-resolution.json" files in all the `__tests__` directories
glob
  .sync(`${__dirname}/../../packages/**/src/**/__tests__/modules-resolution.json`)
  .forEach((file) => {
    // For each of them, merges them in the "mapping" object
    mapping = { ...mapping, ...require(file) };
  });

function resolver(path, options) {
  // If the path corresponds to a key in the mapping object, returns the fakely resolved path
  // otherwise it calls the Jest's default resolver
  return mapping[path] || options.defaultResolver(path, options);
}

module.exports = resolver;
  • Then, I configured it in my jest config file:
module.exports = {
  roots: ['<rootDir>'],
  testMatch: ['<rootDir>/packages/**/__tests__/**/*.test.js'],
  collectCoverageFrom: ['packages/**/src/**/*.js', '!packages/**/src/__tests__/**'],
  resolver: '<rootDir>/test-utils/resolver',
};
  • Finally, I can create modules-resolution.json files in my test folders that look like this:
{
  "fake-module": "/path/to/fake-module"
}

This do the job for me so far and I think that, starting from this example, we could do something more complex but more developer friendly !
Of course, that would be even better if this feature could be directly included in jest !
Anyway, I hope this will help some of you 😉

@vvanpo
Copy link
Contributor

vvanpo commented Aug 16, 2020

I'm a bit hesitant of adding new APIs to Jest because you are running into a very limited use case that not many people are encountering. I think I would prefer simply making it so you can overwrite require.resolve, then you can do whatever you like with it.

@cpojer I think any project that has optional peer dependencies and alters its behaviours based on the existence of those dependencies likely cannot implement full test coverage because require.resolve is not mockable. Such codebases almost certainly have their optional peer dependencies installed as development dependencies, so any require.resolve() call wrapped in a try ... catch will have an untested codepath in the catch block, unless they resort to something hacky like temporarily removing the module from the filesystem, which likely requires consecutive runs of jest and therefore coverage-merging.

@mattvb91
Copy link

Would love to have this functionality too!

My usecase specifically is checking if a directory exists for example require.resolve("@packageName/some-dir/")

@SimenB
Copy link
Member

SimenB commented Oct 18, 2020

The feature has been accepted, so no need to try to convince us 🙂 PR very much welcome!

@kylemh
Copy link

kylemh commented Mar 7, 2021

Hey @SimenB

I read

We have jest.mock('thing', factory, {virtual: true}) if you don't want the thing you're mocking to exist. I think that should work with require.resolve as well, no?

Am I to understand that jest.mock('some-node_module-dep', () => {}, { virtual: true }) should make 'some-node_module-dep' fail to resolve in my source code? I'm trying to add tests to assert that we warn users when we're trying to resolve a peerDep they don't have, and that doesn't seem to be working :(

@SimenB
Copy link
Member

SimenB commented Mar 7, 2021

Correct, that's part of what this issue is about. jest.mock does not affect require.resolve

@github-actions
Copy link

github-actions bot commented Mar 7, 2022

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 Mar 7, 2022
@t1m0thyj
Copy link

This limitation still exists in Jest where it's not possible to mock require.resolve or require.resolve.paths. I vote to keep this issue open.

@github-actions github-actions bot removed the Stale label Mar 21, 2022
@JounQin
Copy link

JounQin commented Aug 29, 2022

I'm trying to use moduleNameMapper for require.resolve, and not quite sure to understand why it does not work.

@SimenB SimenB added the Pinned label Aug 29, 2022
@Tom910
Copy link

Tom910 commented Apr 28, 2023

I found relative problem with @fastify/swagger-ui. This library is using "require.resolve('./static/logo.svg')" and as result test broken with moduleNameMapper configuration

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

10 participants