From d45984f65f610b733e74acff05e2d0effed72c39 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Wed, 30 Dec 2020 16:03:35 +0100 Subject: [PATCH 1/2] feat: resolve reexported CJS modules as named ESM exports --- CHANGELOG.md | 1 + e2e/native-esm/__tests__/native-esm.test.js | 9 ++++- e2e/native-esm/commonjsNamed.cjs | 8 ++++ e2e/native-esm/namedExport.cjs | 1 + packages/jest-runtime/src/index.ts | 41 ++++++++++++++------- 5 files changed, 44 insertions(+), 16 deletions(-) create mode 100644 e2e/native-esm/commonjsNamed.cjs diff --git a/CHANGELOG.md b/CHANGELOG.md index b605adb268c1..2016e121a8d1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -72,6 +72,7 @@ - `[jest-resolve, jest-runtime]` [**BREAKING**] Use `Map`s instead of objects for all cached resources ([#10968](https://github.com/facebook/jest/pull/10968)) - `[jest-runner]` [**BREAKING**] Migrate to ESM ([#10900](https://github.com/facebook/jest/pull/10900)) - `[jest-runtime]` [**BREAKING**] Remove deprecated and unnused `getSourceMapInfo` from Runtime ([#9969](https://github.com/facebook/jest/pull/9969)) +- `[jest-runtime]` Detect reexports from CJS as named exports in ESM ([#10988](https://github.com/facebook/jest/pull/10988)) - `[jest-util]` No longer checking `enumerable` when adding `process.domain` ([#10862](https://github.com/facebook/jest/pull/10862)) - `[jest-validate]` [**BREAKING**] Remove `recursiveBlacklist ` option in favor of previously introduced `recursiveDenylist` ([#10650](https://github.com/facebook/jest/pull/10650)) diff --git a/e2e/native-esm/__tests__/native-esm.test.js b/e2e/native-esm/__tests__/native-esm.test.js index d8ce206e2376..cbc41a136085 100644 --- a/e2e/native-esm/__tests__/native-esm.test.js +++ b/e2e/native-esm/__tests__/native-esm.test.js @@ -14,7 +14,7 @@ import {fileURLToPath} from 'url'; import {jest as jestObject} from '@jest/globals'; import staticImportedStatefulFromCjs from '../fromCjs.mjs'; import {double} from '../index'; -import defaultFromCjs, {namedFunction} from '../namedExport.cjs'; +import defaultFromCjs, {half, namedFunction} from '../namedExport.cjs'; // eslint-disable-next-line import/named import {bag} from '../namespaceExport.js'; import staticImportedStateful from '../stateful.mjs'; @@ -139,10 +139,15 @@ test('varies module cache by query', () => { }); test('supports named imports from CJS', () => { + expect(half(4)).toBe(2); expect(namedFunction()).toBe('hello from a named CJS function!'); expect(defaultFromCjs.default()).toBe('"default" export'); - expect(Object.keys(defaultFromCjs)).toEqual(['namedFunction', 'default']); + expect(Object.keys(defaultFromCjs)).toEqual([ + 'half', + 'namedFunction', + 'default', + ]); }); test('supports file urls as imports', async () => { diff --git a/e2e/native-esm/commonjsNamed.cjs b/e2e/native-esm/commonjsNamed.cjs new file mode 100644 index 000000000000..7017df473e10 --- /dev/null +++ b/e2e/native-esm/commonjsNamed.cjs @@ -0,0 +1,8 @@ +/** + * 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. + */ + +module.exports.half = require('./commonjs.cjs'); diff --git a/e2e/native-esm/namedExport.cjs b/e2e/native-esm/namedExport.cjs index afd9b3edd813..64de6a335c1d 100644 --- a/e2e/native-esm/namedExport.cjs +++ b/e2e/native-esm/namedExport.cjs @@ -5,5 +5,6 @@ * LICENSE file in the root directory of this source tree. */ +module.exports = require('./commonjsNamed.cjs'); module.exports.namedFunction = () => 'hello from a named CJS function!'; module.exports.default = () => '"default" export'; diff --git a/packages/jest-runtime/src/index.ts b/packages/jest-runtime/src/index.ts index 1c720944581c..04bcaff6242c 100644 --- a/packages/jest-runtime/src/index.ts +++ b/packages/jest-runtime/src/index.ts @@ -524,21 +524,15 @@ export default class Runtime { // CJS loaded via `import` should share cache with other CJS: https://github.com/nodejs/modules/issues/503 const cjs = this.requireModuleOrMock(from, modulePath); - const transformedCode = this._fileTransforms.get(modulePath); + const parsedExports = this.getExportsOfCjs(modulePath); - let cjsExports: ReadonlyArray = []; - - if (transformedCode) { - const {exports} = parseCjs(transformedCode.code); - - cjsExports = exports.filter(exportName => { - // we don't wanna respect any exports _names_ default as a named export - if (exportName === 'default') { - return false; - } - return Object.hasOwnProperty.call(cjs, exportName); - }); - } + const cjsExports = [...parsedExports].filter(exportName => { + // we don't wanna respect any exports _named_ default as a named export + if (exportName === 'default') { + return false; + } + return Object.hasOwnProperty.call(cjs, exportName); + }); const module = new SyntheticModule( [...cjsExports, 'default'], @@ -556,6 +550,25 @@ export default class Runtime { return evaluateSyntheticModule(module); } + private getExportsOfCjs(modulePath: Config.Path) { + const transformedCode = + this._fileTransforms.get(modulePath)?.code ?? this.readFile(modulePath); + + const {exports, reexports} = parseCjs(transformedCode); + + let namedExports = new Set(exports); + + reexports.forEach(reexport => { + const resolved = this._resolveModule(modulePath, reexport); + + const exports = this.getExportsOfCjs(resolved); + + namedExports = new Set([...namedExports, ...exports]); + }); + + return namedExports; + } + requireModule( from: Config.Path, moduleName?: string, From 688f0aa873495042996a269e791d89073f7d59a0 Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Wed, 30 Dec 2020 17:24:40 +0100 Subject: [PATCH 2/2] cache lookups --- packages/jest-runtime/src/index.ts | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/packages/jest-runtime/src/index.ts b/packages/jest-runtime/src/index.ts index 04bcaff6242c..a4992f045f4f 100644 --- a/packages/jest-runtime/src/index.ts +++ b/packages/jest-runtime/src/index.ts @@ -165,7 +165,8 @@ export default class Runtime { private readonly _moduleMocker: ModuleMocker; private _isolatedModuleRegistry: ModuleRegistry | null; private _moduleRegistry: ModuleRegistry; - private readonly _esmoduleRegistry: Map; + private readonly _esmoduleRegistry: Map; + private readonly _cjsNamedExports: Map>; private readonly _testPath: Config.Path; private readonly _resolver: Resolver; private _shouldAutoMock: boolean; @@ -214,6 +215,7 @@ export default class Runtime { this._isolatedMockRegistry = null; this._moduleRegistry = new Map(); this._esmoduleRegistry = new Map(); + this._cjsNamedExports = new Map(); this._testPath = testPath; this._resolver = resolver; this._scriptTransformer = new ScriptTransformer(config, this._cacheFS); @@ -551,21 +553,29 @@ export default class Runtime { } private getExportsOfCjs(modulePath: Config.Path) { + const cachedNamedExports = this._cjsNamedExports.get(modulePath); + + if (cachedNamedExports) { + return cachedNamedExports; + } + const transformedCode = this._fileTransforms.get(modulePath)?.code ?? this.readFile(modulePath); const {exports, reexports} = parseCjs(transformedCode); - let namedExports = new Set(exports); + const namedExports = new Set(exports); reexports.forEach(reexport => { const resolved = this._resolveModule(modulePath, reexport); const exports = this.getExportsOfCjs(resolved); - namedExports = new Set([...namedExports, ...exports]); + exports.forEach(namedExports.add, namedExports); }); + this._cjsNamedExports.set(modulePath, namedExports); + return namedExports; } @@ -858,6 +868,7 @@ export default class Runtime { this._mockRegistry.clear(); this._moduleRegistry.clear(); this._esmoduleRegistry.clear(); + this._cjsNamedExports.clear(); if (this._environment) { if (this._environment.global) {