From 3559c3a7f59365735e677e0bbec99d570fc8fd40 Mon Sep 17 00:00:00 2001 From: takerusilt Date: Wed, 2 Dec 2020 03:20:03 +0800 Subject: [PATCH] fix: unable to cyclic ES modules (#10892) --- CHANGELOG.md | 1 + .../__snapshots__/nativeEsm.test.ts.snap | 2 +- e2e/native-esm/__tests__/native-esm.test.js | 7 ++ e2e/native-esm/circularDependentA.mjs | 15 ++++ e2e/native-esm/circularDependentB.mjs | 15 ++++ packages/jest-runtime/src/index.ts | 74 ++++++++++++++----- 6 files changed, 96 insertions(+), 18 deletions(-) create mode 100644 e2e/native-esm/circularDependentA.mjs create mode 100644 e2e/native-esm/circularDependentB.mjs diff --git a/CHANGELOG.md b/CHANGELOG.md index f5ef3be2f2f3..5eb449be3668 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,7 @@ - `[jest-resolve]` Disable `jest-pnp-resolver` for Yarn 2 ([#10847](https://github.com/facebook/jest/pull/10847)) - `[jest-runtime]` [**BREAKING**] Do not inject `global` variable into module wrapper ([#10644](https://github.com/facebook/jest/pull/10644)) - `[jest-runtime]` [**BREAKING**] remove long-deprecated `jest.addMatchers`, `jest.resetModuleRegistry`, and `jest.runTimersToTime` ([#9853](https://github.com/facebook/jest/pull/9853)) +- `[jest-runtime]` Fix stack overflow and promise deadlock when importing mutual dependant ES module ([#10892](https://github.com/facebook/jest/pull/10892)) - `[jest-transform]` Show enhanced `SyntaxError` message for all `SyntaxError`s ([#10749](https://github.com/facebook/jest/pull/10749)) - `[jest-transform]` [**BREAKING**] Refactor API to pass an options bag around rather than multiple boolean options ([#10753](https://github.com/facebook/jest/pull/10753)) - `[jest-transform]` [**BREAKING**] Refactor API of transformers to pass an options bag rather than separate `config` and other options diff --git a/e2e/__tests__/__snapshots__/nativeEsm.test.ts.snap b/e2e/__tests__/__snapshots__/nativeEsm.test.ts.snap index f336c89b96aa..b1e95ff54928 100644 --- a/e2e/__tests__/__snapshots__/nativeEsm.test.ts.snap +++ b/e2e/__tests__/__snapshots__/nativeEsm.test.ts.snap @@ -10,7 +10,7 @@ Ran all test suites matching /native-esm.tla.test.js/i. exports[`on node ^12.16.0 || >=13.7.0 runs test with native ESM 1`] = ` Test Suites: 1 passed, 1 total -Tests: 17 passed, 17 total +Tests: 18 passed, 18 total Snapshots: 0 total Time: <> Ran all test suites matching /native-esm.test.js/i. diff --git a/e2e/native-esm/__tests__/native-esm.test.js b/e2e/native-esm/__tests__/native-esm.test.js index 019f0ff2f340..f46a9ab57cd4 100644 --- a/e2e/native-esm/__tests__/native-esm.test.js +++ b/e2e/native-esm/__tests__/native-esm.test.js @@ -156,3 +156,10 @@ test('supports file urls as imports', async () => { test('namespace export', () => { expect(bag.double).toBe(double); }); + +test('handle circular dependency', async () => { + const moduleA = (await import('../circularDependentA.mjs')).default; + expect(moduleA.id).toBe('circularDependentA'); + expect(moduleA.moduleB.id).toBe('circularDependentB'); + expect(moduleA.moduleB.moduleA).toBe(moduleA); +}); diff --git a/e2e/native-esm/circularDependentA.mjs b/e2e/native-esm/circularDependentA.mjs new file mode 100644 index 000000000000..801bd8d8443d --- /dev/null +++ b/e2e/native-esm/circularDependentA.mjs @@ -0,0 +1,15 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import circularDependentB from './circularDependentB.mjs'; + +export default { + id: 'circularDependentA', + get moduleB() { + return circularDependentB; + }, +}; diff --git a/e2e/native-esm/circularDependentB.mjs b/e2e/native-esm/circularDependentB.mjs new file mode 100644 index 000000000000..7a81d4c331c6 --- /dev/null +++ b/e2e/native-esm/circularDependentB.mjs @@ -0,0 +1,15 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +import circularDependentA from './circularDependentA.mjs'; + +export default { + id: 'circularDependentB', + get moduleA() { + return circularDependentA; + }, +}; diff --git a/packages/jest-runtime/src/index.ts b/packages/jest-runtime/src/index.ts index 22d8c46ca4f0..d7c7544df7d0 100644 --- a/packages/jest-runtime/src/index.ts +++ b/packages/jest-runtime/src/index.ts @@ -57,6 +57,11 @@ import type {Context} from './types'; export type {Context} from './types'; +interface EsmModuleCache { + beforeEvaluation: Promise; + fullyEvaluated: Promise; +} + const esmIsAvailable = typeof SourceTextModule === 'function'; interface JestGlobals extends Global.TestFrameworkGlobals { @@ -172,7 +177,7 @@ export default class Runtime { private _moduleMocker: ModuleMocker; private _isolatedModuleRegistry: ModuleRegistry | null; private _moduleRegistry: ModuleRegistry; - private _esmoduleRegistry: Map>; + private _esmoduleRegistry: Map; private _testPath: Config.Path | undefined; private _resolver: Resolver; private _shouldAutoMock: boolean; @@ -363,6 +368,7 @@ export default class Runtime { private async loadEsmModule( modulePath: Config.Path, query = '', + isStaticImport = false, ): Promise { const cacheKey = modulePath + query; @@ -378,7 +384,10 @@ export default class Runtime { if (this._resolver.isCoreModule(modulePath)) { const core = this._importCoreModule(modulePath, context); - this._esmoduleRegistry.set(cacheKey, core); + this._esmoduleRegistry.set(cacheKey, { + beforeEvaluation: core, + fullyEvaluated: core, + }); return core; } @@ -401,31 +410,56 @@ export default class Runtime { specifier, referencingModule.identifier, referencingModule.context, + false, ), initializeImportMeta(meta: ImportMeta) { meta.url = pathToFileURL(modulePath).href; }, }); + let resolve: (value: VMModule) => void; + let reject: (value: any) => void; + const promise = new Promise((_resolve, _reject) => { + resolve = _resolve; + reject = _reject; + }); + + // add to registry before link so that circular import won't end up stack overflow this._esmoduleRegistry.set( cacheKey, // we wanna put the linking promise in the cache so modules loaded in // parallel can all await it. We then await it synchronously below, so // we shouldn't get any unhandled rejections - module - .link((specifier: string, referencingModule: VMModule) => - this.linkModules( - specifier, - referencingModule.identifier, - referencingModule.context, - ), - ) - .then(() => module.evaluate()) - .then(() => module), + { + beforeEvaluation: Promise.resolve(module), + fullyEvaluated: promise, + }, ); + + module + .link((specifier: string, referencingModule: VMModule) => + this.linkModules( + specifier, + referencingModule.identifier, + referencingModule.context, + true, + ), + ) + .then(() => module.evaluate()) + .then( + () => resolve(module), + (e: any) => reject(e), + ); } - const module = this._esmoduleRegistry.get(cacheKey); + const entry = this._esmoduleRegistry.get(cacheKey); + + // return the already resolved, pre-evaluation promise + // is loaded through static import to prevent promise deadlock + // because module is evaluated after all static import is resolved + const module = isStaticImport + ? entry?.beforeEvaluation + : entry?.fullyEvaluated; invariant(module); @@ -436,15 +470,21 @@ export default class Runtime { specifier: string, referencingIdentifier: string, context: VMContext, + isStaticImport: boolean, ) { if (specifier === '@jest/globals') { const fromCache = this._esmoduleRegistry.get('@jest/globals'); if (fromCache) { - return fromCache; + return isStaticImport + ? fromCache.beforeEvaluation + : fromCache.fullyEvaluated; } const globals = this.getGlobalsForEsm(referencingIdentifier, context); - this._esmoduleRegistry.set('@jest/globals', globals); + this._esmoduleRegistry.set('@jest/globals', { + beforeEvaluation: globals, + fullyEvaluated: globals, + }); return globals; } @@ -461,7 +501,7 @@ export default class Runtime { this._resolver.isCoreModule(resolved) || this.unstable_shouldLoadAsEsm(resolved) ) { - return this.loadEsmModule(resolved, query); + return this.loadEsmModule(resolved, query, isStaticImport); } return this.loadCjsAsEsm(referencingIdentifier, resolved, context); @@ -1169,7 +1209,7 @@ export default class Runtime { invariant(context); - return this.linkModules(specifier, scriptFilename, context); + return this.linkModules(specifier, scriptFilename, context, false); }, }); } catch (e) {