chore(transform): refactor API to pass an options bag around rather t…

…han multiple boolean options (#10753)
jestjs · Nov 4, 2020 · ab5bd0f · ab5bd0f
1 parent 30b6cee
commit ab5bd0f
Show file tree

Hide file tree

Showing 5 changed files with 45 additions and 78 deletions.
diff --git 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
@@ -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
@@ -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,22 +452,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;
     }
@@ -500,14 +471,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
@@ -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
@@ -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
@@ -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();
   });