Disable the class fields transform when disableESTransforms is set (#625

)

When disableESTransforms is specified, the class transform can almost entirely
be skipped, but we still need to do some work:
* TS constructor initializers (e.g. constructor params with `readonly` or
  `private`) need to be transformed to assignments within the constructor.
* Fields marked `declare` need to be removed. Uninitialized fields not marked
  declare are preserved, matching the ES spec and the `useDefineForClassFields`
  flag in TS.

For now, we still use the `getClassInfo` code path but give alternate results.
In the future we can hopefully simplify that code path, and possibly change
`declare` parsing so the whole line consists of type tokens that will naturally
be erased.

src/parser/plugins/flow.ts

@@ -698,6 +698,7 @@ function flowParseTypeAnnotatableIdentifier(): void {
 export function flowParseVariance(): void {
   if (match(tt.plus) || match(tt.minus)) {
     next();
+    state.tokens[state.tokens.length - 1].isType = true;
   }
 }
 

src/transformers/RootTransformer.ts

@@ -25,6 +25,7 @@ export default class RootTransformer {
   private generatedVariables: Array<string> = [];
   private isImportsTransformEnabled: boolean;
   private isReactHotLoaderTransformEnabled: boolean;
+  private disableESTransforms: boolean;
   private helperManager: HelperManager;
 
   constructor(
@@ -39,6 +40,7 @@ export default class RootTransformer {
     this.tokens = tokenProcessor;
     this.isImportsTransformEnabled = transforms.includes("imports");
     this.isReactHotLoaderTransformEnabled = transforms.includes("react-hot-loader");
+    this.disableESTransforms = Boolean(options.disableESTransforms);
 
     if (!options.disableESTransforms) {
       this.transformers.push(
@@ -193,7 +195,7 @@ export default class RootTransformer {
   }
 
   processClass(): void {
-    const classInfo = getClassInfo(this, this.tokens, this.nameManager);
+    const classInfo = getClassInfo(this, this.tokens, this.nameManager, this.disableESTransforms);
 
     // Both static and instance initializers need a class name to use to invoke the initializer, so
     // assign to one if necessary.

src/util/getClassInfo.ts

@@ -49,6 +49,7 @@ export default function getClassInfo(
   rootTransformer: RootTransformer,
   tokens: TokenProcessor,
   nameManager: NameManager,
+  disableESTransforms: boolean,
 ): ClassInfo {
   const snapshot = tokens.snapshot();
 
@@ -71,7 +72,9 @@ export default function getClassInfo(
     if (tokens.matchesContextual(ContextualKeyword._constructor) && !tokens.currentToken().isType) {
       ({constructorInitializerStatements, constructorInsertPos} = processConstructor(tokens));
     } else if (tokens.matches1(tt.semi)) {
-      rangesToRemove.push({start: tokens.currentIndex(), end: tokens.currentIndex() + 1});
+      if (!disableESTransforms) {
+        rangesToRemove.push({start: tokens.currentIndex(), end: tokens.currentIndex() + 1});
+      }
       tokens.nextToken();
     } else if (tokens.currentToken().isType) {
       tokens.nextToken();
@@ -80,13 +83,17 @@ export default function getClassInfo(
       const statementStartIndex = tokens.currentIndex();
       let isStatic = false;
       let isESPrivate = false;
+      let isDeclare = false;
       while (isAccessModifier(tokens.currentToken())) {
         if (tokens.matches1(tt._static)) {
           isStatic = true;
         }
         if (tokens.matches1(tt.hash)) {
           isESPrivate = true;
         }
+        if (tokens.matches1(tt._declare)) {
+          isDeclare = true;
+        }
         tokens.nextToken();
       }
       if (isStatic && tokens.matches1(tt.braceL)) {
@@ -144,23 +151,47 @@ export default function getClassInfo(
           start: nameStartIndex,
           end: tokens.currentIndex(),
         });
-      } else {
-        // This is just a declaration, so doesn't need to produce any code in the output.
+      } else if (!disableESTransforms || isDeclare) {
+        // This is a regular field declaration, like `x;`. With the class transform enabled, we just
+        // remove the line so that no output is produced. With the class transform disabled, we
+        // usually want to preserve the declaration (but still strip types), but if the `declare`
+        // keyword is specified, we should remove the line to avoid initializing the value to
+        // undefined.
         rangesToRemove.push({start: statementStartIndex, end: tokens.currentIndex()});
       }
     }
   }
 
   tokens.restoreToSnapshot(snapshot);
-  return {
-    headerInfo,
-    constructorInitializerStatements,
-    instanceInitializerNames,
-    staticInitializerNames,
-    constructorInsertPos,
-    fields,
-    rangesToRemove,
-  };
+  if (disableESTransforms) {
+    // With ES transforms disabled, we don't want to transform regular class
+    // field declarations, and we don't need to do any additional tricks to
+    // reference the constructor for static init, but we still need to transform
+    // TypeScript field initializers defined as constructor parameters and we
+    // still need to remove `declare` fields. For now, we run the same code
+    // path but omit any field information, as if the class had no field
+    // declarations. In the future, when we fully drop the class fields
+    // transform, we can simplify this code significantly.
+    return {
+      headerInfo,
+      constructorInitializerStatements,
+      instanceInitializerNames: [],
+      staticInitializerNames: [],
+      constructorInsertPos,
+      fields: [],
+      rangesToRemove,
+    };
+  } else {
+    return {
+      headerInfo,
+      constructorInitializerStatements,
+      instanceInitializerNames,
+      staticInitializerNames,
+      constructorInsertPos,
+      fields,
+      rangesToRemove,
+    };
+  }
 }
 
 /**

test/flow-test.ts

@@ -1,15 +1,23 @@
 import {throws} from "assert";
 
-import {transform} from "../src";
+import {transform, Options} from "../src";
 import {IMPORT_DEFAULT_PREFIX} from "./prefixes";
 import {assertResult} from "./util";
 
-function assertFlowResult(code: string, expectedResult: string): void {
-  assertResult(code, expectedResult, {transforms: ["jsx", "imports", "flow"]});
+function assertFlowResult(
+  code: string,
+  expectedResult: string,
+  options: Partial<Options> = {},
+): void {
+  assertResult(code, expectedResult, {transforms: ["jsx", "imports", "flow"], ...options});
 }
 
-function assertFlowESMResult(code: string, expectedResult: string): void {
-  assertResult(code, expectedResult, {transforms: ["jsx", "flow"]});
+function assertFlowESMResult(
+  code: string,
+  expectedResult: string,
+  options: Partial<Options> = {},
+): void {
+  assertResult(code, expectedResult, {transforms: ["jsx", "flow"], ...options});
 }
 
 describe("transform flow", () => {
@@ -427,4 +435,22 @@ describe("transform flow", () => {
     `,
     );
   });
+
+  it("properly removes class property variance markers with ES transforms disabled", () => {
+    assertFlowResult(
+      `
+      class C {
+        +foo: number;
+        -bar: number;
+      }
+    `,
+      `"use strict";
+      class C {
+        foo;
+        bar;
+      }
+    `,
+      {disableESTransforms: true},
+    );
+  });
 });

test/sucrase-test.ts

@@ -1363,4 +1363,40 @@ describe("sucrase", () => {
       {transforms: [], disableESTransforms: true},
     );
   });
+
+  it("skips class field transform when ES transforms are disabled", () => {
+    assertResult(
+      `
+      class A {
+        x = 1;
+        static y = 2;
+      }
+    `,
+      `
+      class A {
+        x = 1;
+        static y = 2;
+      }
+    `,
+      {transforms: [], disableESTransforms: true},
+    );
+  });
+
+  it("skips class field transform for expression classes when ES transforms are disabled", () => {
+    assertResult(
+      `
+      C = class {
+        x = 1;
+        static y = 2;
+      }
+    `,
+      `
+      C = class {
+        x = 1;
+        static y = 2;
+      }
+    `,
+      {transforms: [], disableESTransforms: true},
+    );
+  });
 });

test/typescript-test.ts

@@ -1,3 +1,4 @@
+import type {Options} from "../src/Options";
 import {
   CREATE_STAR_EXPORT_PREFIX,
   ESMODULE_PREFIX,
@@ -8,12 +9,20 @@ import {
 } from "./prefixes";
 import {assertResult, devProps} from "./util";
 
-function assertTypeScriptResult(code: string, expectedResult: string): void {
-  assertResult(code, expectedResult, {transforms: ["jsx", "imports", "typescript"]});
+function assertTypeScriptResult(
+  code: string,
+  expectedResult: string,
+  options: Partial<Options> = {},
+): void {
+  assertResult(code, expectedResult, {transforms: ["jsx", "imports", "typescript"], ...options});
 }
 
-function assertTypeScriptESMResult(code: string, expectedResult: string): void {
-  assertResult(code, expectedResult, {transforms: ["jsx", "typescript"]});
+function assertTypeScriptESMResult(
+  code: string,
+  expectedResult: string,
+  options: Partial<Options> = {},
+): void {
+  assertResult(code, expectedResult, {transforms: ["jsx", "typescript"], ...options});
 }
 
 function assertTypeScriptImportResult(
@@ -2448,4 +2457,72 @@ describe("typescript transform", () => {
     `,
     );
   });
+
+  it("transforms constructor initializers even with disableESTransforms", () => {
+    assertTypeScriptESMResult(
+      `
+      class A extends B {
+        constructor(readonly x, private y = 2, z: number = 3) {
+          console.log("Hello");
+          super();
+          console.log("World");
+        }
+      }
+    `,
+      `
+      class A extends B {
+        constructor( x,  y = 2, z = 3) {
+          console.log("Hello");
+          super();this.x = x;this.y = y;;
+          console.log("World");
+        }
+      }
+    `,
+      {disableESTransforms: true},
+    );
+  });
+
+  it("removes types from class fields with disableESTransforms", () => {
+    assertTypeScriptESMResult(
+      `
+      class A {
+        x: number = 3;
+        static y: string = "Hello";
+        z: boolean;
+        static s: unknown;
+      }
+    `,
+      `
+      class A {
+        x = 3;
+        static y = "Hello";
+        z;
+        static s;
+      }
+    `,
+      {disableESTransforms: true},
+    );
+  });
+
+  it("removes declare fields with disableESTransforms", () => {
+    assertTypeScriptESMResult(
+      `
+      class A {
+        abstract readonly a: number = 3;
+        declare b: string;
+        declare static c: string;
+        static declare d: string;
+      }
+    `,
+      `
+      class A {
+          a = 3;
+        ;
+        ;
+        ;
+      }
+    `,
+      {disableESTransforms: true},
+    );
+  });
 });