From 596ebff371f99f29ebff8904dbe62fdd42330f4c Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Wed, 30 Dec 2020 18:04:41 +0100 Subject: [PATCH] feat: resolve reexported CJS modules as named ESM exports (#10988) --- 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 | 54 +++++++++++++++------ 5 files changed, 56 insertions(+), 17 deletions(-) create mode 100644 e2e/native-esm/commonjsNamed.cjs diff --git a/CHANGELOG.md b/CHANGELOG.md index ea94197a0ad7..1d655334ef71 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -73,6 +73,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..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); @@ -524,21 +526,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 +552,33 @@ export default class Runtime { return evaluateSyntheticModule(module); } + 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); + + const namedExports = new Set(exports); + + reexports.forEach(reexport => { + const resolved = this._resolveModule(modulePath, reexport); + + const exports = this.getExportsOfCjs(resolved); + + exports.forEach(namedExports.add, namedExports); + }); + + this._cjsNamedExports.set(modulePath, namedExports); + + return namedExports; + } + requireModule( from: Config.Path, moduleName?: string, @@ -845,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) {