From 59c265300d67aa1e9b7824ff5f84e74beaf6089c Mon Sep 17 00:00:00 2001 From: Simen Bekkhus Date: Sat, 31 Oct 2020 16:25:41 +0100 Subject: [PATCH] chore(transform): refactor API to pass an options bag around rather than multiple boolean options --- CHANGELOG.md | 1 + .../src/generateEmptyCoverage.ts | 2 +- .../jest-transform/src/ScriptTransformer.ts | 108 ++++++------------ .../script_transformer.test.ts.snap | 6 +- .../src/__tests__/script_transformer.test.ts | 6 +- 5 files changed, 45 insertions(+), 78 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ef1f8926045a..f4ac32ed83c0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ ### Fixes - `[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)) ### Chore & Maintenance diff --git a/packages/jest-reporters/src/generateEmptyCoverage.ts b/packages/jest-reporters/src/generateEmptyCoverage.ts index a56b5100d253..cc23ba8c0339 100644 --- a/packages/jest-reporters/src/generateEmptyCoverage.ts +++ b/packages/jest-reporters/src/generateEmptyCoverage.ts @@ -70,7 +70,7 @@ export default function ( const {code} = new ScriptTransformer(config).transformSource( filename, source, - true, + {instrument: true}, ); // TODO: consider passing AST const extracted = readInitialCoverage(code); diff --git a/packages/jest-transform/src/ScriptTransformer.ts b/packages/jest-transform/src/ScriptTransformer.ts index 921d9c22669c..f99310ab3911 100644 --- a/packages/jest-transform/src/ScriptTransformer.ts +++ b/packages/jest-transform/src/ScriptTransformer.ts @@ -28,6 +28,7 @@ import handlePotentialSyntaxError from './enhanceUnexpectedTokenMessage'; import shouldInstrument from './shouldInstrument'; import type { Options, + TransformOptions, TransformResult, TransformedSource, Transformer, @@ -92,9 +93,7 @@ export default class ScriptTransformer { private _getCacheKey( fileData: string, filename: Config.Path, - instrument: boolean, - supportsDynamicImport: boolean, - supportsStaticESM: boolean, + options: TransformOptions, ): string { const configString = this._cache.configString; const transformer = this._getTransformer(filename); @@ -104,10 +103,12 @@ export default class ScriptTransformer { .update( transformer.getCacheKey(fileData, filename, configString, { config: this._config, - instrument, + instrument: options.instrument, rootDir: this._config.rootDir, - supportsDynamicImport, - supportsStaticESM, + supportsDynamicImport: options.supportsDynamicImport, + supportsExportNamespaceFrom: options.supportsExportNamespaceFrom, + supportsStaticESM: options.supportsStaticESM, + supportsTopLevelAwait: options.supportsTopLevelAwait, }), ) .update(CACHE_VERSION) @@ -116,7 +117,7 @@ export default class ScriptTransformer { return createHash('md5') .update(fileData) .update(configString) - .update(instrument ? 'instrument' : '') + .update(options.instrument ? 'instrument' : '') .update(filename) .update(CACHE_VERSION) .digest('hex'); @@ -126,22 +127,14 @@ export default class ScriptTransformer { private _getFileCachePath( filename: Config.Path, content: string, - instrument: boolean, - supportsDynamicImport: boolean, - supportsStaticESM: boolean, + options: TransformOptions, ): Config.Path { const baseCacheDir = HasteMap.getCacheFilePath( this._config.cacheDirectory, 'jest-transform-cache-' + this._config.name, VERSION, ); - const cacheKey = this._getCacheKey( - content, - filename, - instrument, - supportsDynamicImport, - supportsStaticESM, - ); + const cacheKey = this._getCacheKey(content, filename, options); // Create sub folders based on the cacheKey to avoid creating one // directory with many files. const cacheDir = path.join(baseCacheDir, cacheKey[0] + cacheKey[1]); @@ -212,9 +205,8 @@ export default class ScriptTransformer { private _instrumentFile( filename: Config.Path, input: TransformedSource, - supportsDynamicImport: boolean, - supportsStaticESM: boolean, canMapToInput: boolean, + options: TransformOptions, ): TransformedSource { const inputCode = typeof input === 'string' ? input : input.code; const inputMap = typeof input === 'string' ? null : input.map; @@ -224,8 +216,10 @@ export default class ScriptTransformer { babelrc: false, caller: { name: '@jest/transform', - supportsDynamicImport, - supportsStaticESM, + supportsDynamicImport: options.supportsDynamicImport, + supportsExportNamespaceFrom: options.supportsExportNamespaceFrom, + supportsStaticESM: options.supportsStaticESM, + supportsTopLevelAwait: options.supportsTopLevelAwait, }, configFile: false, filename, @@ -259,23 +253,14 @@ export default class ScriptTransformer { this._getTransformer(filepath); } - // TODO: replace third argument with TransformOptions in Jest 26 transformSource( filepath: Config.Path, content: string, - instrument: boolean, - supportsDynamicImport = false, - supportsStaticESM = false, + options: TransformOptions, ): TransformResult { const filename = tryRealpath(filepath); const transform = this._getTransformer(filename); - const cacheFilePath = this._getFileCachePath( - filename, - content, - instrument, - supportsDynamicImport, - supportsStaticESM, - ); + const cacheFilePath = this._getFileCachePath(filename, content, options); let sourceMapPath: Config.Path | null = cacheFilePath + '.map'; // Ignore cache if `config.cache` is set (--no-cache) let code = this._config.cache ? readCodeCacheFile(cacheFilePath) : null; @@ -305,11 +290,12 @@ export default class ScriptTransformer { }; if (transform && shouldCallTransform) { - const processed = transform.process(content, filename, this._config, { - instrument, - supportsDynamicImport, - supportsStaticESM, - }); + const processed = transform.process( + content, + filename, + this._config, + options, + ); if (typeof processed === 'string') { transformed.code = processed; @@ -343,7 +329,7 @@ export default class ScriptTransformer { // Apply instrumentation to the code if necessary, keeping the instrumented code and new map let map = transformed.map; - if (!transformWillInstrument && instrument) { + if (!transformWillInstrument && options.instrument) { /** * We can map the original source code to the instrumented code ONLY if * - the process of transforming the code produced a source map e.g. ts-jest @@ -359,9 +345,8 @@ export default class ScriptTransformer { const instrumented = this._instrumentFile( filename, transformed, - supportsDynamicImport, - supportsStaticESM, shouldEmitSourceMaps, + options, ); code = @@ -391,15 +376,10 @@ export default class ScriptTransformer { private _transformAndBuildScript( filename: Config.Path, options: Options, - instrument: boolean, + transformOptions: TransformOptions, fileSource?: string, ): TransformResult { - const { - isCoreModule, - isInternalModule, - supportsDynamicImport, - supportsStaticESM, - } = options; + const {isCoreModule, isInternalModule} = options; const content = stripShebang( fileSource || fs.readFileSync(filename, 'utf8'), ); @@ -410,16 +390,14 @@ export default class ScriptTransformer { const willTransform = !isInternalModule && !isCoreModule && - (this.shouldTransform(filename) || instrument); + (transformOptions.instrument || this.shouldTransform(filename)); try { if (willTransform) { const transformedSource = this.transformSource( filename, content, - instrument, - supportsDynamicImport, - supportsStaticESM, + transformOptions, ); code = transformedSource.code; @@ -458,7 +436,7 @@ export default class ScriptTransformer { const result = this._transformAndBuildScript( filename, options, - instrument, + {...options, instrument}, fileSource, ); @@ -474,12 +452,7 @@ export default class ScriptTransformer { options: Options, fileSource: string, ): string { - const { - isCoreModule, - isInternalModule, - supportsDynamicImport, - supportsStaticESM, - } = options; + const {isCoreModule, isInternalModule} = options; const willTransform = !isInternalModule && !isCoreModule && this.shouldTransform(filename); @@ -487,9 +460,7 @@ export default class ScriptTransformer { const {code: transformedJsonSource} = this.transformSource( filename, fileSource, - false, - supportsDynamicImport, - supportsStaticESM, + {...options, instrument: false}, ); return transformedJsonSource; } @@ -500,14 +471,17 @@ export default class ScriptTransformer { requireAndTranspileModule( moduleName: string, callback?: (module: ModuleType) => void, + transformOptions?: TransformOptions, ): ModuleType; requireAndTranspileModule( moduleName: string, callback?: (module: ModuleType) => Promise, + transformOptions?: TransformOptions, ): Promise; requireAndTranspileModule( moduleName: string, callback?: (module: ModuleType) => void | Promise, + transformOptions: TransformOptions = {instrument: false}, ): ModuleType | Promise { // Load the transformer to avoid a cycle where we need to load a // transformer in order to transform it in the require hooks @@ -519,9 +493,7 @@ export default class ScriptTransformer { try { transforming = true; return ( - // we might wanna do `supportsDynamicImport` at some point - this.transformSource(filename, code, false, false, false).code || - code + this.transformSource(filename, code, transformOptions).code || code ); } finally { transforming = false; @@ -562,14 +534,6 @@ export default class ScriptTransformer { return module; } - /** - * @deprecated use `this.shouldTransform` instead - */ - // @ts-expect-error: Unused and private - remove in Jest 25 - private _shouldTransform(filename: Config.Path): boolean { - return this.shouldTransform(filename); - } - shouldTransform(filename: Config.Path): boolean { const ignoreRegexp = this._cache.ignorePatternsRegExp; const isIgnored = ignoreRegexp ? ignoreRegexp.test(filename) : false; diff --git a/packages/jest-transform/src/__tests__/__snapshots__/script_transformer.test.ts.snap b/packages/jest-transform/src/__tests__/__snapshots__/script_transformer.test.ts.snap index e5ad6e704386..c9021bd66fb9 100644 --- a/packages/jest-transform/src/__tests__/__snapshots__/script_transformer.test.ts.snap +++ b/packages/jest-transform/src/__tests__/__snapshots__/script_transformer.test.ts.snap @@ -71,8 +71,10 @@ Object { }, "instrument": true, "rootDir": "/", - "supportsDynamicImport": false, - "supportsStaticESM": false, + "supportsDynamicImport": undefined, + "supportsExportNamespaceFrom": undefined, + "supportsStaticESM": undefined, + "supportsTopLevelAwait": undefined, } `; diff --git a/packages/jest-transform/src/__tests__/script_transformer.test.ts b/packages/jest-transform/src/__tests__/script_transformer.test.ts index 3615f8fee634..998c07d4579b 100644 --- a/packages/jest-transform/src/__tests__/script_transformer.test.ts +++ b/packages/jest-transform/src/__tests__/script_transformer.test.ts @@ -338,7 +338,7 @@ describe('ScriptTransformer', () => { }; const scriptTransformer = new ScriptTransformer(config); expect(() => - scriptTransformer.transformSource('sample.js', '', false), + scriptTransformer.transformSource('sample.js', '', {instrument: false}), ).toThrow('Jest: a transform must export a `process` function.'); }); @@ -355,7 +355,7 @@ describe('ScriptTransformer', () => { }; const scriptTransformer = new ScriptTransformer(config); expect(() => - scriptTransformer.transformSource('sample.js', '', false), + scriptTransformer.transformSource('sample.js', '', {instrument: false}), ).toThrow('Jest: a transform must export a `process` function.'); }); @@ -366,7 +366,7 @@ describe('ScriptTransformer', () => { }; const scriptTransformer = new ScriptTransformer(config); expect(() => - scriptTransformer.transformSource('sample.js', '', false), + scriptTransformer.transformSource('sample.js', '', {instrument: false}), ).not.toThrow(); });