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

feat(runtime): add jest.mockModule #10976

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Conversation

SimenB
Copy link
Member

@SimenB SimenB commented Dec 23, 2020

Summary

A start on #10025. Lots of stuff missing

  • Manual mocks (i.e. mocks from __mocks__ directories)
  • Docs
  • mocks without factories - not sure how to load them and determine types etc.
  • behavior when calling mock and doMock on an ES Module, and mockImport on CJS. I think it should throw?
  • jest.importActual
  • handle concurrent imports of mocks. Right now there's a race condition since it doesn't populate the cache immediately upon creating a mock. Need some sort of mutex

Test plan

I'll add more tests as we go

@damianobarbati
Copy link

@SimenB as you said in #9430 (comment) this is needed to implement the exports feature: do you know of any workaround meanwhile?

@SimenB
Copy link
Member Author

SimenB commented Feb 9, 2021

@SimenB as you said in #9430 (comment) this is needed to implement the exports feature: do you know of any workaround meanwhile?

This is not related to exports, what I meant is that that is the final missing piece. Not dependent on the feature in this PR. You can track exports support in #9771

@codecov-io
Copy link

Codecov Report

Merging #10976 (eac6673) into master (a03b6fe) will decrease coverage by 0.10%.
The diff coverage is 30.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10976      +/-   ##
==========================================
- Coverage   64.28%   64.17%   -0.11%     
==========================================
  Files         308      308              
  Lines       13480    13512      +32     
  Branches     3286     3291       +5     
==========================================
+ Hits         8665     8671       +6     
- Misses       4106     4134      +28     
+ Partials      709      707       -2     
Impacted Files Coverage Δ
packages/jest-resolve/src/index.ts 45.96% <0.00%> (ø)
packages/jest-runtime/src/index.ts 53.15% <30.95%> (-1.70%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a03b6fe...eac6673. Read the comment docs.

@piranna
Copy link
Contributor

piranna commented Jun 2, 2021

what's the state of this?

@SimenB
Copy link
Member Author

SimenB commented Jun 3, 2021

I currently don't have the motivation to work on this feature, so it's in hiatus until I get back to it. Others are of course free to pick it up (missing stuff is mostly laid out in the OP from what I remember) 🙂 I can rebase this, tho

@codecov-commenter
Copy link

codecov-commenter commented Jun 3, 2021

Codecov Report

Merging #10976 (9af07a9) into master (90d6908) will decrease coverage by 0.01%.
The diff coverage is 14.28%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10976      +/-   ##
==========================================
- Coverage   68.93%   68.92%   -0.02%     
==========================================
  Files         312      312              
  Lines       16398    16401       +3     
  Branches     4749     4750       +1     
==========================================
  Hits        11304    11304              
- Misses       5067     5070       +3     
  Partials       27       27              
Impacted Files Coverage Δ
packages/jest-runtime/src/index.ts 57.12% <14.28%> (-0.21%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 90d6908...9af07a9. Read the comment docs.

@connorjclark
Copy link
Contributor

@SimenB, is there a natural point to split up this PR / what remains? For example, if __mocks__ or jest.importActual is a significant amount of work, would it be preferable for a contributor to work on the rest first and land those things in a follow up PR?

I can take a look at helping with some of this tomorrow. A rebase, if it'd be quick for you, would be appreciated (assuming there might be conflicts to be resolved that I wouldn't have the context for).

@SimenB
Copy link
Member Author

SimenB commented Aug 27, 2021

@connorjclark awesome if you wanna pick this up! I just rebased it. Not sure where the best point to pick something up would be. I assume e.g. jest.importActual should be fairly self-contained and not too advanced, so should be a nice place to start 🙂

@benmccann
Copy link
Contributor

Some of the items from above look required to merge this, but I wonder if some might be able to be moved to follow-ups? E.g. this could provide a lot of value without manual mocks support

@SimenB
Copy link
Member Author

SimenB commented Sep 7, 2021

Sure, pretty much every point could be moved to a follow-up except possibly docs... Although one could argue docs can wait until the feature is more stabilized. I could add this as jest.unstable_mockModule or something for now to be safe, and remove the prefix when it's feature complete (and then the unstable function in the next major)

@benmccann
Copy link
Contributor

Checking it in as jest.unstable_mockModule would be awesome and very much appreciated!

@SimenB
Copy link
Member Author

SimenB commented Sep 7, 2021

@benmccann landed that now, will release tonight or tomorrow (cest)

@SimenB
Copy link
Member Author

SimenB commented Sep 8, 2021

Out in https://github.com/facebook/jest/releases/tag/v27.1.1

@benmccann
Copy link
Contributor

Thank you so much!

@n0mer
Copy link

n0mer commented Jan 2, 2022

@SimenB time to update https://jestjs.io/docs/ecmascript-modules , it still says "Please note that we currently don't support jest.mock in a clean way in ESM, but that is something we intend to add proper support for in the future. Follow this issue for updates."

@SimenB
Copy link
Member Author

SimenB commented Jan 5, 2022

@n0mer if you're up for a quick PR adding a small note that'd be awesome 👍 Can link to #9430 (comment) or something for the caveats

@cbouwense
Copy link

Are there any plans to continue this work in the near term? @SimenB

@SimenB
Copy link
Member Author

SimenB commented Mar 8, 2022

All work on ESM is paused until most issues linked in nodejs/node#37648 are solved.

(I got a message from a developer at Google last week about movement, so hopefully that'll actually move along very soon)

@SimenB
Copy link
Member Author

SimenB commented Mar 8, 2022

That said, happy to take PRs of course. But any personal effort is on pause until node's APIs are closer to stabilization.

@oldmansutton
Copy link

The downside to jest.unstable_mockModule() right now is that it mocks the entire module. This is maybe not a downside if that's what you're looking for. It's really falling down for me right now though, because I only want to mock a few functions from the module. This really needs a jest.importActual() to fill in that missing gap. Trying to use jest.requireActual() on an ESM of course returns a 'Must use import to load ES Module' error, which one would expect to happen. I'm getting to the point where even I might take a crack at implementing that functionality, but

a) For the past 26 years I've written in pure javascript for use in a browser environment (hence why I'm using ESM)
b) Am not super familiar with TypeScript outside of Haxe because I've always been able to do what I need to in pure JavaScript (And transpiling/bundling feels dirty since it's supposed to be a runtime script)
c) Am not super familiar with Node outside of using npm to pull down libraries, as the server side has always been Perl/PHP/C#/Python up until the last few years. Of course this all means that using Jest has been a struggle learning opportunity.

At the very least it looks like the requireActual() implementation is happening in node_modules/jest-runtime/build/index.js, and that it is basically just calling requireModule() with a flag flipped to true. The flag being flipped to true seems to only really effect if modulePath = manualMock or not, which then effects if you receive the mocked module or the actual module through black magic Node CJS shennanigans transforms.

From this I would guess that within the same file, the unstable_importModule() function (or further down the line loadEsmModule() function) could be extended to accept the same sort of flag solution. However, once we get there, there is even more Pure ES Standards Javascript to CJS/Babel transform voodoo to decipher. When I get to this point it starts bringing on too many Bad Memories of the JavaScript vs. JScript days and I get triggered and have to leave the room and have a lie down.

I guess if anybody was to take a crack at jest.importActual that's the place to do it (or wherever jest-runtime/build/index.js is actually transpiled down from). Unfortunately, it's probably not going to be me, because ironically I find pure JavaScript easy (again, 26 years of practice), but find the modern workflow bloated and overly complicated. However, I hope somebody who understands the newer way of doing things finds the research helpful, or like Simen says, we wait for Node to support the standards and it doesn't take another few years.

@revmischa

This comment was marked as off-topic.

@github-actions
Copy link

This PR is stale because it has been open 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 Jun 15, 2023
@SimenB SimenB added Pinned and removed Stale labels Jun 21, 2023
rekmarks added a commit to MetaMask/create-release-branch that referenced this pull request Dec 8, 2023
This PR updates `execa` to `^8.0.1`. Since `execa@>=6.0.0` is ESM-only and `jest` only has experimental ESM support (jestjs/jest#10976), this required switching from `ts-jest` to `babel-jest`. To minimize dependency transpilation, the ESM packages that are necessary to transpile are enumerated in `jest.config.js`.

This version of `execa` includes [automatic escaping of shell arguments](https://github.com/sindresorhus/execa/tree/v8.0.1#execafile-arguments-options), which was the entire point of #112, #113, and this PR.

The state of ESM support in the Node.js ecosystem is absolutely horrible, and I would not recommend further migrations for the time being. We should continue to dual-release our packages and avoid ESM-only dependencies until the ecosystem has matured. For details see the above `jest` issue and nodejs/node#37648.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.