From 3512f5726b8bbe8522bb1301a9b5646385dfc227 Mon Sep 17 00:00:00 2001 From: Michael Loughry Date: Fri, 5 Apr 2019 12:13:45 -0700 Subject: [PATCH 1/9] Allow JSON transforms for #2578 --- packages/jest-runtime/src/index.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/jest-runtime/src/index.ts b/packages/jest-runtime/src/index.ts index 86e7115f9860..81e676968009 100644 --- a/packages/jest-runtime/src/index.ts +++ b/packages/jest-runtime/src/index.ts @@ -464,7 +464,11 @@ class Runtime { options: InternalModuleOptions | undefined, moduleRegistry: ModuleRegistry, ) { - if (path.extname(modulePath) === '.json') { + if ( + path.extname(modulePath) === '.json' && + options && + options.isInternalModule + ) { localModule.exports = this._environment.global.JSON.parse( stripBOM(fs.readFileSync(modulePath, 'utf8')), ); From 2777f66b05e511cae23c2e9351b9b1a84a13d0e1 Mon Sep 17 00:00:00 2001 From: Michael Loughry Date: Fri, 5 Apr 2019 12:17:06 -0700 Subject: [PATCH 2/9] Additional comments --- CHANGELOG.md | 1 + packages/jest-runtime/src/index.ts | 3 +++ 2 files changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cbbb653666e..c88c88c7d777 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### Features ### Fixes +- `[@jest/runtime]` Allow custom transforms for JSON dependencies ### Chore & Maintenance diff --git a/packages/jest-runtime/src/index.ts b/packages/jest-runtime/src/index.ts index 81e676968009..a3df4587d18c 100644 --- a/packages/jest-runtime/src/index.ts +++ b/packages/jest-runtime/src/index.ts @@ -466,6 +466,9 @@ class Runtime { ) { if ( path.extname(modulePath) === '.json' && + // We still need to fallback to this method for internal modules, + // as `ci-info` ends up throwing an exception when it fails to + // import a JSON file correctly options && options.isInternalModule ) { From 94e5493e2f8dc1b9295be6fb7a1da19f5ef80f84 Mon Sep 17 00:00:00 2001 From: Michael Loughry Date: Fri, 5 Apr 2019 13:41:47 -0700 Subject: [PATCH 3/9] A better fix, with test --- .../__tests__/runtime_internal_module.test.js | 24 +++++++++++++++++++ .../__tests__/test_root/internal-root.json | 3 +++ .../test_root/test_json_preprocessor.js | 10 ++++++++ packages/jest-runtime/src/index.ts | 14 +---------- .../jest-transform/src/ScriptTransformer.ts | 9 +++++-- 5 files changed, 45 insertions(+), 15 deletions(-) create mode 100644 packages/jest-runtime/src/__tests__/test_root/internal-root.json create mode 100644 packages/jest-runtime/src/__tests__/test_root/test_json_preprocessor.js diff --git a/packages/jest-runtime/src/__tests__/runtime_internal_module.test.js b/packages/jest-runtime/src/__tests__/runtime_internal_module.test.js index b434b0994804..7ed833b9f4a9 100644 --- a/packages/jest-runtime/src/__tests__/runtime_internal_module.test.js +++ b/packages/jest-runtime/src/__tests__/runtime_internal_module.test.js @@ -44,5 +44,29 @@ describe('Runtime', () => { const exports = runtime.requireInternalModule(modulePath); expect(exports()).toBe('internal-module-data'); })); + + it('loads JSON modules and applies transforms', () => + createRuntime(__filename, { + transform: {'^.+\\.json$': './test_json_preprocessor'}, + }).then(runtime => { + const modulePath = path.resolve( + path.dirname(runtime.__mockRootPath), + 'internal-root.json', + ); + const exports = runtime.requireModule(modulePath); + expect(exports).toEqual({foo: 'foo'}); + })); + + it('loads internal JSON modules without applying transforms', () => + createRuntime(__filename, { + transform: {'^.+\\.json$': './test_json_preprocessor'}, + }).then(runtime => { + const modulePath = path.resolve( + path.dirname(runtime.__mockRootPath), + 'internal-root.json', + ); + const exports = runtime.requireInternalModule(modulePath); + expect(exports).toEqual({foo: 'bar'}); + })); }); }); diff --git a/packages/jest-runtime/src/__tests__/test_root/internal-root.json b/packages/jest-runtime/src/__tests__/test_root/internal-root.json new file mode 100644 index 000000000000..c8c4105eb57c --- /dev/null +++ b/packages/jest-runtime/src/__tests__/test_root/internal-root.json @@ -0,0 +1,3 @@ +{ + "foo": "bar" +} diff --git a/packages/jest-runtime/src/__tests__/test_root/test_json_preprocessor.js b/packages/jest-runtime/src/__tests__/test_root/test_json_preprocessor.js new file mode 100644 index 000000000000..0a260ab97c64 --- /dev/null +++ b/packages/jest-runtime/src/__tests__/test_root/test_json_preprocessor.js @@ -0,0 +1,10 @@ +/** + * 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. + */ + +'use strict'; + +module.exports.process = () => `{"foo": "foo"}`; diff --git a/packages/jest-runtime/src/index.ts b/packages/jest-runtime/src/index.ts index a3df4587d18c..97bc6f2cb227 100644 --- a/packages/jest-runtime/src/index.ts +++ b/packages/jest-runtime/src/index.ts @@ -27,7 +27,6 @@ import { shouldInstrument, } from '@jest/transform'; import fs from 'graceful-fs'; -import stripBOM from 'strip-bom'; import {run as cliRun} from './cli'; import {options as cliOptions} from './cli/args'; import {findSiblingsWithFileExtension} from './helpers'; @@ -464,18 +463,7 @@ class Runtime { options: InternalModuleOptions | undefined, moduleRegistry: ModuleRegistry, ) { - if ( - path.extname(modulePath) === '.json' && - // We still need to fallback to this method for internal modules, - // as `ci-info` ends up throwing an exception when it fails to - // import a JSON file correctly - options && - options.isInternalModule - ) { - localModule.exports = this._environment.global.JSON.parse( - stripBOM(fs.readFileSync(modulePath, 'utf8')), - ); - } else if (path.extname(modulePath) === '.node') { + if (path.extname(modulePath) === '.node') { localModule.exports = require(modulePath); } else { // Only include the fromPath if a moduleName is given. Else treat as root. diff --git a/packages/jest-transform/src/ScriptTransformer.ts b/packages/jest-transform/src/ScriptTransformer.ts index e7267362c981..5a753b70697e 100644 --- a/packages/jest-transform/src/ScriptTransformer.ts +++ b/packages/jest-transform/src/ScriptTransformer.ts @@ -331,6 +331,8 @@ export default class ScriptTransformer { !isInternalModule && !isCoreModule && (this.shouldTransform(filename) || instrument); + const isJson = path.extname(filename) === '.json'; + const prefix = isJson ? 'module.exports = ' : ''; try { const extraGlobals = (options && options.extraGlobals) || []; @@ -342,11 +344,14 @@ export default class ScriptTransformer { instrument, ); - wrappedCode = wrap(transformedSource.code, ...extraGlobals); + wrappedCode = wrap( + `${prefix}${transformedSource.code}`, + ...extraGlobals, + ); sourceMapPath = transformedSource.sourceMapPath; mapCoverage = transformedSource.mapCoverage; } else { - wrappedCode = wrap(content, ...extraGlobals); + wrappedCode = wrap(`${prefix}${content}`, ...extraGlobals); } return { From 469b3635b961fc879ab3589d558ae6f414226865 Mon Sep 17 00:00:00 2001 From: Michael Loughry Date: Fri, 5 Apr 2019 15:05:37 -0700 Subject: [PATCH 4/9] Fix linting error from CI runs --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c88c88c7d777..0792d8ad606a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ### Features ### Fixes + - `[@jest/runtime]` Allow custom transforms for JSON dependencies ### Chore & Maintenance From ad380b40e03569b0bc5129e67a5bd93b689bd7fe Mon Sep 17 00:00:00 2001 From: Michael Loughry Date: Fri, 5 Apr 2019 15:30:30 -0700 Subject: [PATCH 5/9] Address comment from @scotthovestadt --- .../src/__tests__/test_root/test_json_preprocessor.js | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/jest-runtime/src/__tests__/test_root/test_json_preprocessor.js b/packages/jest-runtime/src/__tests__/test_root/test_json_preprocessor.js index 0a260ab97c64..ec9b48f69cc2 100644 --- a/packages/jest-runtime/src/__tests__/test_root/test_json_preprocessor.js +++ b/packages/jest-runtime/src/__tests__/test_root/test_json_preprocessor.js @@ -7,4 +7,8 @@ 'use strict'; -module.exports.process = () => `{"foo": "foo"}`; +module.exports.process = source => { + const json = JSON.parse(source); + Object.keys(json).forEach(k => (json[k] = k)); + return JSON.stringify(json); +}; From b4c5a1f4824cc4c67c5419973bc150b76c748f40 Mon Sep 17 00:00:00 2001 From: OWA Framework Date: Sat, 6 Apr 2019 10:22:41 -0700 Subject: [PATCH 6/9] Re-implement per @scotthovestadt's guidance --- packages/jest-runtime/src/index.ts | 15 ++++++++- .../jest-transform/src/ScriptTransformer.ts | 31 ++++++++++++++----- 2 files changed, 38 insertions(+), 8 deletions(-) diff --git a/packages/jest-runtime/src/index.ts b/packages/jest-runtime/src/index.ts index 97bc6f2cb227..8963fb61b19f 100644 --- a/packages/jest-runtime/src/index.ts +++ b/packages/jest-runtime/src/index.ts @@ -27,6 +27,7 @@ import { shouldInstrument, } from '@jest/transform'; import fs from 'graceful-fs'; +import stripBOM from 'strip-bom'; import {run as cliRun} from './cli'; import {options as cliOptions} from './cli/args'; import {findSiblingsWithFileExtension} from './helpers'; @@ -463,7 +464,19 @@ class Runtime { options: InternalModuleOptions | undefined, moduleRegistry: ModuleRegistry, ) { - if (path.extname(modulePath) === '.node') { + if (path.extname(modulePath) === '.json') { + const text = stripBOM(fs.readFileSync(modulePath, 'utf8')); + + const transformedFile = this._scriptTransformer.transformJson( + modulePath, + options, + text, + ); + + localModule.exports = this._environment.global.JSON.parse( + transformedFile, + ); + } else if (path.extname(modulePath) === '.node') { localModule.exports = require(modulePath); } else { // Only include the fromPath if a moduleName is given. Else treat as root. diff --git a/packages/jest-transform/src/ScriptTransformer.ts b/packages/jest-transform/src/ScriptTransformer.ts index 5a753b70697e..2375ce60b4e6 100644 --- a/packages/jest-transform/src/ScriptTransformer.ts +++ b/packages/jest-transform/src/ScriptTransformer.ts @@ -331,8 +331,6 @@ export default class ScriptTransformer { !isInternalModule && !isCoreModule && (this.shouldTransform(filename) || instrument); - const isJson = path.extname(filename) === '.json'; - const prefix = isJson ? 'module.exports = ' : ''; try { const extraGlobals = (options && options.extraGlobals) || []; @@ -344,14 +342,11 @@ export default class ScriptTransformer { instrument, ); - wrappedCode = wrap( - `${prefix}${transformedSource.code}`, - ...extraGlobals, - ); + wrappedCode = wrap(transformedSource.code, ...extraGlobals); sourceMapPath = transformedSource.sourceMapPath; mapCoverage = transformedSource.mapCoverage; } else { - wrappedCode = wrap(`${prefix}${content}`, ...extraGlobals); + wrappedCode = wrap(content, ...extraGlobals); } return { @@ -410,6 +405,28 @@ export default class ScriptTransformer { return result; } + transformJson( + filename: Config.Path, + options: Pick | undefined, + fileSource: string, + ): string { + const isInternalModule = !!(options && options.isInternalModule); + const isCoreModule = !!(options && options.isCoreModule); + const willTransform = + !isInternalModule && !isCoreModule && this.shouldTransform(filename); + + if (willTransform) { + const {code: transformedJsonSource} = this.transformSource( + filename, + fileSource, + false, + ); + return transformedJsonSource; + } + + return fileSource; + } + /** * @deprecated use `this.shouldTransform` instead */ From 8a023da311011cca942a55132386f78e706669f7 Mon Sep 17 00:00:00 2001 From: OWA Framework Date: Sat, 6 Apr 2019 11:17:53 -0700 Subject: [PATCH 7/9] Update Changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d856e9891ae..3a52d4d2a057 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,7 @@ ### Fixes - `[jest-snapshot]` Inline snapshots: do not indent empty lines ([#8277](https://github.com/facebook/jest/pull/8277)) -- `[@jest/runtime]` Allow custom transforms for JSON dependencies ([#2578](https://github.com/facebook/jest/pull/2578)) +- `[@jest/runtime, @jest/transform]` Allow custom transforms for JSON dependencies ([#2578](https://github.com/facebook/jest/pull/2578)) ### Chore & Maintenance From b46f305b4f160b4eb70926da117c1fd50ff763e3 Mon Sep 17 00:00:00 2001 From: Michael Loughry Date: Sat, 6 Apr 2019 15:27:48 -0700 Subject: [PATCH 8/9] Provide full options to transformJson --- packages/jest-runtime/src/index.ts | 26 ++++++++++++------- .../jest-transform/src/ScriptTransformer.ts | 6 ++--- packages/jest-transform/src/index.ts | 6 ++++- 3 files changed, 24 insertions(+), 14 deletions(-) diff --git a/packages/jest-runtime/src/index.ts b/packages/jest-runtime/src/index.ts index 8963fb61b19f..84f944f99a89 100644 --- a/packages/jest-runtime/src/index.ts +++ b/packages/jest-runtime/src/index.ts @@ -25,6 +25,7 @@ import { ScriptTransformer, ShouldInstrumentOptions, shouldInstrument, + TransformationOptions, } from '@jest/transform'; import fs from 'graceful-fs'; import stripBOM from 'strip-bom'; @@ -469,7 +470,7 @@ class Runtime { const transformedFile = this._scriptTransformer.transformJson( modulePath, - options, + this._getFullTransformationOptions(options), text, ); @@ -486,6 +487,19 @@ class Runtime { localModule.loaded = true; } + private _getFullTransformationOptions( + options: InternalModuleOptions | undefined, + ): TransformationOptions { + return { + ...options, + extraGlobals: this._config.extraGlobals || [], + changedFiles: this._coverageOptions.changedFiles, + collectCoverage: this._coverageOptions.collectCoverage, + collectCoverageFrom: this._coverageOptions.collectCoverageFrom, + collectCoverageOnlyFrom: this._coverageOptions.collectCoverageOnlyFrom, + }; + } + requireModuleOrMock(from: Config.Path, moduleName: string) { try { if (this._shouldMock(from, moduleName)) { @@ -684,7 +698,6 @@ class Runtime { return; } - const isInternalModule = !!(options && options.isInternalModule); const filename = localModule.filename; const lastExecutingModulePath = this._currentlyExecutingModulePath; this._currentlyExecutingModulePath = filename; @@ -709,14 +722,7 @@ class Runtime { const extraGlobals = this._config.extraGlobals || []; const transformedFile = this._scriptTransformer.transform( filename, - { - changedFiles: this._coverageOptions.changedFiles, - collectCoverage: this._coverageOptions.collectCoverage, - collectCoverageFrom: this._coverageOptions.collectCoverageFrom, - collectCoverageOnlyFrom: this._coverageOptions.collectCoverageOnlyFrom, - extraGlobals, - isInternalModule, - }, + this._getFullTransformationOptions(options), this._cacheFS[filename], ); diff --git a/packages/jest-transform/src/ScriptTransformer.ts b/packages/jest-transform/src/ScriptTransformer.ts index 2375ce60b4e6..3e0e7c4d8b93 100644 --- a/packages/jest-transform/src/ScriptTransformer.ts +++ b/packages/jest-transform/src/ScriptTransformer.ts @@ -407,11 +407,11 @@ export default class ScriptTransformer { transformJson( filename: Config.Path, - options: Pick | undefined, + options: Options, fileSource: string, ): string { - const isInternalModule = !!(options && options.isInternalModule); - const isCoreModule = !!(options && options.isCoreModule); + const isInternalModule = options.isInternalModule; + const isCoreModule = options.isCoreModule; const willTransform = !isInternalModule && !isCoreModule && this.shouldTransform(filename); diff --git a/packages/jest-transform/src/index.ts b/packages/jest-transform/src/index.ts index 4a757f5ad9fd..c5be0650c891 100644 --- a/packages/jest-transform/src/index.ts +++ b/packages/jest-transform/src/index.ts @@ -7,4 +7,8 @@ export {default as ScriptTransformer} from './ScriptTransformer'; export {default as shouldInstrument} from './shouldInstrument'; -export {Transformer, ShouldInstrumentOptions} from './types'; +export { + Transformer, + ShouldInstrumentOptions, + Options as TransformationOptions, +} from './types'; From c36d878b09a5181f7d41f74ed79e54fcfd3eefc5 Mon Sep 17 00:00:00 2001 From: Michael Loughry Date: Sat, 6 Apr 2019 16:34:01 -0700 Subject: [PATCH 9/9] Fix lint error --- packages/jest-runtime/src/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/jest-runtime/src/index.ts b/packages/jest-runtime/src/index.ts index 84f944f99a89..7307c79c9bb1 100644 --- a/packages/jest-runtime/src/index.ts +++ b/packages/jest-runtime/src/index.ts @@ -492,11 +492,11 @@ class Runtime { ): TransformationOptions { return { ...options, - extraGlobals: this._config.extraGlobals || [], changedFiles: this._coverageOptions.changedFiles, collectCoverage: this._coverageOptions.collectCoverage, collectCoverageFrom: this._coverageOptions.collectCoverageFrom, collectCoverageOnlyFrom: this._coverageOptions.collectCoverageOnlyFrom, + extraGlobals: this._config.extraGlobals || [], }; }