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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

feature: jest.isolateModules accept an async function for dynamic import()s #10428

Closed
vvanpo opened this issue Aug 20, 2020 · 7 comments 路 Fixed by #13680
Closed

feature: jest.isolateModules accept an async function for dynamic import()s #10428

vvanpo opened this issue Aug 20, 2020 · 7 comments 路 Fixed by #13680

Comments

@vvanpo
Copy link
Contributor

vvanpo commented Aug 20, 2020

馃殌 Feature Proposal

Have the ability to pass jest.isolatedModules an async function, and have it wait for the promise to resolve before cleaning up the isolated module registry.

I think this feature might be kind of tricky, because currently it appears there is at most one isolated module registry in a given runtime, meaning that successive calls to jest.isolatedModules with async functions (without waiting for each to resolve) would overlap and fail.

Motivation

Right now, jest.isolateModules() appears to work for static imports only: https://github.com/facebook/jest/blob/v26.4.0/packages/jest-runtime/src/index.ts#L748. But if you use babel-jest and your modules make use of dynamic import()s, the finally block will clean up the isolated modules registry prior to import()s resolving.

My specific use-case for this feature is in defining a helper function for integration tests, that sets up my entire application鈥攎ocking some key dependencies鈥攁nd returns a reference to the root page component mounted in the jsdom environment. This entrypoint uses multiple dynamic imports during its initialization. I'd like the helper function to be self-contained, as I can't make assumptions about how it's called. It could be called multiple times within a test suite, for example, hence the desire to use isolatedModules.

Example

  async function loadApp() {
    document.body.innerHTML = '<div id="#app"></div>' // The mount point required by the entrypoint.
    let component

    await jest.isolatedModules(async () => {  // If passed an async function, `isolatedModules` should likewise return a promise.
      jest.doMock('depA', ... )
      ...

      component = await require('path/to/app/entrypoint') // The entrypoint exports a promise resolving to the root component instance, once all initialization is complete.
    })

    return component
  }
  test('App mounts to #app', async () => {
    const vm = await loadApp()
    const mountpoint = document.querySelector('#app')

    expect(vm.el).toBe(mountpoint)
  })
@maxammann
Copy link

Especially useful if you are forced to use isolateModules because of this: facebook/react#16012

@SimenB
Copy link
Member

SimenB commented Sep 16, 2020

Yeah, this (or something similar) is needed for proper ESM support

@dreyks
Copy link

dreyks commented May 11, 2021

is this something that is still looked into? having async support for isolateModules would be great. Each of my projects has the following patch so that I can use it with import()s

The change is pretty straightforward, do you think we can incorporate this into the codebase? I can make a proper PR with test coverage and stuff

diff --git a/node_modules/jest-runtime/build/index.js b/node_modules/jest-runtime/build/index.js
index 38c6753..a901ea8 100644
--- a/node_modules/jest-runtime/build/index.js
+++ b/node_modules/jest-runtime/build/index.js
@@ -924,7 +924,7 @@ class Runtime {
     }
   }
 
-  isolateModules(fn) {
+  async isolateModules(fn) {
     if (this._isolatedModuleRegistry || this._isolatedMockRegistry) {
       throw new Error(
         'isolateModules cannot be nested inside another isolateModules.'
@@ -935,7 +935,7 @@ class Runtime {
     this._isolatedMockRegistry = new Map();
 
     try {
-      fn();
+      await fn();
     } finally {
       var _this$_isolatedModule, _this$_isolatedMockRe;
 
@@ -1741,8 +1741,8 @@ class Runtime {
       return jestObject;
     };
 
-    const isolateModules = fn => {
-      this.isolateModules(fn);
+    const isolateModules = async fn => {
+      await this.isolateModules(fn);
       return jestObject;
     };

this of course forces you to await isolateModules in your code if you want to access the jestObject but I think it's ok. if it's not ok, however, we can make two different top-level functions isolateModules and isolateModulesAsync that will delegate to the same internal isolaleModules implementation

@corysimmons
Copy link

@SimenB Would you merge @dreyks change if someone opened a PR?

@mmanciop
Copy link
Contributor

mmanciop commented Dec 19, 2022

@SimenB Would you merge @dreyks change if someone opened a PR?

I decided to find out :D
#13680

@SimenB
Copy link
Member

SimenB commented Jan 24, 2023

https://github.com/facebook/jest/releases/tag/v29.4.0

@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 Feb 24, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants