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

fix: unable to cyclic ES modules #10892

Merged
merged 4 commits into from Dec 1, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion e2e/__tests__/__snapshots__/nativeEsm.test.ts.snap
Expand Up @@ -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: <<REPLACED>>
Ran all test suites matching /native-esm.test.js/i.
Expand Down
7 changes: 7 additions & 0 deletions e2e/native-esm/__tests__/native-esm.test.js
Expand Up @@ -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);
});
15 changes: 15 additions & 0 deletions 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;
},
};
15 changes: 15 additions & 0 deletions 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;
},
};
74 changes: 57 additions & 17 deletions packages/jest-runtime/src/index.ts
Expand Up @@ -57,6 +57,11 @@ import type {Context} from './types';

export type {Context} from './types';

interface EsmModuleCache {
beforeEvaluation: Promise<VMModule>;
fullyEvaluated: Promise<VMModule>;
}

const esmIsAvailable = typeof SourceTextModule === 'function';

interface JestGlobals extends Global.TestFrameworkGlobals {
Expand Down Expand Up @@ -172,7 +177,7 @@ export default class Runtime {
private _moduleMocker: typeof jestMock;
private _isolatedModuleRegistry: ModuleRegistry | null;
private _moduleRegistry: ModuleRegistry;
private _esmoduleRegistry: Map<string, Promise<VMModule>>;
private _esmoduleRegistry: Map<string, EsmModuleCache>;
private _testPath: Config.Path | undefined;
private _resolver: Resolver;
private _shouldAutoMock: boolean;
Expand Down Expand Up @@ -363,6 +368,7 @@ export default class Runtime {
private async loadEsmModule(
modulePath: Config.Path,
query = '',
isStaticImport = false,
): Promise<VMModule> {
const cacheKey = modulePath + query;

Expand All @@ -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;
}

Expand All @@ -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<VMModule>((_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);

Expand All @@ -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;
}
Expand All @@ -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);
Expand Down Expand Up @@ -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) {
Expand Down