chore(transform): refactor API to pass an options bag around rather than multiple boolean options

SimenB · SimenB · commit 4ece96b2f4e1 · 2020-10-31T17:40:58.000+01:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -23,6 +23,7 @@
 - `[eslint-config-fb-strict]` Move package from this repo to `fbjs` repo ([#10739](https://github.com/facebook/jest/pull/10739))
 - `[examples]` Update TypeScript example to show use of newer Jest types ([#10399](https://github.com/facebook/jest/pull/10399))
 - `[jest-cli]` chore: standardize files and folder names ([#10698](https://github.com/facebook/jest/pull/10698))
+- `[jest-transform]` [**BREAKING**] Refactor API to pass an options bag around rather than multiple boolean options ([#10753](https://github.com/facebook/jest/pull/10753))
 
 ### Performance
 
diff --git 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
@@ -26,6 +26,7 @@ import {sync as writeFileAtomic} from 'write-file-atomic';
 import {addHook} from 'pirates';
 import type {
   Options,
+  TransformOptions,
   TransformResult,
   TransformedSource,
   Transformer,
@@ -62,6 +63,14 @@ async function waitForPromiseWithCleanup(
   }
 }
 
+// https://github.com/DefinitelyTyped/DefinitelyTyped/pull/49267
+declare module '@babel/core' {
+  interface TransformCaller {
+    supportsExportNamespaceFrom?: boolean;
+    supportsTopLevelAwait?: boolean;
+  }
+}
+
 export default class ScriptTransformer {
   private _cache: ProjectCache;
   private _config: Config.ProjectConfig;
@@ -93,9 +102,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);
@@ -105,10 +112,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)
@@ -117,7 +126,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');
@@ -127,22 +136,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]);
@@ -213,9 +214,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;
@@ -225,8 +225,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,
@@ -260,23 +262,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;
@@ -306,11 +299,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;
@@ -344,7 +338,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
@@ -360,9 +354,8 @@ export default class ScriptTransformer {
       const instrumented = this._instrumentFile(
         filename,
         transformed,
-        supportsDynamicImport,
-        supportsStaticESM,
         shouldEmitSourceMaps,
+        options,
       );
 
       code =
@@ -392,15 +385,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'),
     );
@@ -411,16 +399,14 @@ export default class ScriptTransformer {
     const willTransform =
       !isInternalModule &&
       !isCoreModule &&
-      (this.shouldTransform(filename) || instrument);
+      (this.shouldTransform(filename) || transformOptions.instrument);
 
     try {
       if (willTransform) {
         const transformedSource = this.transformSource(
           filename,
           content,
-          instrument,
-          supportsDynamicImport,
-          supportsStaticESM,
+          transformOptions,
         );
 
         code = transformedSource.code;
@@ -459,7 +445,7 @@ export default class ScriptTransformer {
     const result = this._transformAndBuildScript(
       filename,
       options,
-      instrument,
+      {...options, instrument},
       fileSource,
     );
 
@@ -475,22 +461,15 @@ 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);
 
     if (willTransform) {
       const {code: transformedJsonSource} = this.transformSource(
         filename,
         fileSource,
-        false,
-        supportsDynamicImport,
-        supportsStaticESM,
+        {...options, instrument: false},
       );
       return transformedJsonSource;
     }
@@ -501,14 +480,17 @@ export default class ScriptTransformer {
   requireAndTranspileModule<ModuleType = unknown>(
     moduleName: string,
     callback?: (module: ModuleType) => void,
+    transformOptions?: TransformOptions,
   ): ModuleType;
   requireAndTranspileModule<ModuleType = unknown>(
     moduleName: string,
     callback?: (module: ModuleType) => Promise<void>,
+    transformOptions?: TransformOptions,
   ): Promise<ModuleType>;
   requireAndTranspileModule<ModuleType = unknown>(
     moduleName: string,
     callback?: (module: ModuleType) => void | Promise<void>,
+    transformOptions: TransformOptions = {instrument: false},
   ): ModuleType | Promise<ModuleType> {
     // Load the transformer to avoid a cycle where we need to load a
     // transformer in order to transform it in the require hooks
@@ -520,9 +502,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;
diff --git a/packages/jest-transform/src/__tests__/__snapshots__/script_transformer.test.js.snap b/packages/jest-transform/src/__tests__/__snapshots__/script_transformer.test.js.snap
@@ -70,8 +70,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.js b/packages/jest-transform/src/__tests__/script_transformer.test.js
@@ -473,9 +473,7 @@ describe('ScriptTransformer', () => {
 
     const result = scriptTransformer.transform(
       '/fruits/banana.js',
-      makeGlobalConfig({
-        collectCoverage: true,
-      }),
+      makeGlobalConfig({collectCoverage: true}),
     );
     expect(result.sourceMapPath).toBeNull();
     expect(writeFileAtomic.sync).toBeCalledTimes(1);
@@ -532,9 +530,7 @@ describe('ScriptTransformer', () => {
 
     const result = scriptTransformer.transform(
       '/fruits/banana.js',
-      makeGlobalConfig({
-        collectCoverage: true,
-      }),
+      makeGlobalConfig({collectCoverage: true}),
     );
     expect(result.sourceMapPath).toBeFalsy();
     expect(writeFileAtomic.sync).toHaveBeenCalledTimes(1);
@@ -571,9 +567,7 @@ describe('ScriptTransformer', () => {
 
     const result = scriptTransformer.transform(
       '/fruits/banana.js',
-      makeGlobalConfig({
-        collectCoverage: true,
-      }),
+      makeGlobalConfig({collectCoverage: true}),
     );
     expect(result.sourceMapPath).toEqual(expect.any(String));
     expect(writeFileAtomic.sync).toBeCalledTimes(2);
@@ -609,9 +603,7 @@ describe('ScriptTransformer', () => {
 
     const result = scriptTransformer.transform(
       '/fruits/banana.js',
-      makeGlobalConfig({
-        collectCoverage: true,
-      }),
+      makeGlobalConfig({collectCoverage: true}),
     );
     expect(result.sourceMapPath).toEqual(expect.any(String));
     expect(writeFileAtomic.sync).toBeCalledTimes(2);
@@ -631,9 +623,7 @@ describe('ScriptTransformer', () => {
 
     scriptTransformer.transform(
       '/fruits/banana.js',
-      makeGlobalConfig({
-        collectCoverage: true,
-      }),
+      makeGlobalConfig({collectCoverage: true}),
     );
 
     const {getCacheKey} = require('test_preprocessor');
