Fix const enum exports

Align const enum exports with TypeScript. That is: - preserveConstEnums is disabled --> only export the type, runtime values are elided - preserveConstEnums is enabled --> export the value if the const enum is declared in a .ts file from the same compilation unit; otherwise export type only When is preserveConstEnums enabled, isolatedModules is likely enabled, too. isolatedModules prevents const enum usage when the enum comes from .d.ts files. Star re-exports are allowed, though. It's possible the enum is then imported from a compilation unit that does not use isolatedModules. That unit needs the type info, so it's important to always export the type. But that unit won't access the value because const enums are inlined. PiperOrigin-RevId: 601770954
angular · Jan 26, 2024 · 96f1cb3 · 96f1cb3
1 parent d5c2921
commit 96f1cb3
Show file tree

Hide file tree

Showing 55 changed files with 1,303 additions and 574 deletions.
diff --git a/demo/package.json b/demo/package.json
@@ -8,7 +8,7 @@
   "dependencies": {
     "minimist": "^1.2.3",
     "tsickle": "file:../",
-    "typescript": "5.2.2"
+    "typescript": "5.3.2"
   },
   "devDependencies": {
     "@types/minimist": "1.2.0",

diff --git a/package.json b/package.json
@@ -11,7 +11,7 @@
     "out/src/*"
   ],
   "peerDependencies": {
-    "typescript": "~5.1.5"
+    "typescript": "~5.3.2"
   },
   "devDependencies": {
     "@types/diff-match-patch": "^1.0.32",
@@ -28,7 +28,7 @@
     "source-map-support": "^0.5.19",
     "tslib": "^2.2.0",
     "tslint": "^6.1.3",
-    "typescript": "5.2.2"
+    "typescript": "5.3.2"
   },
   "scripts": {
     "build": "tsc",

diff --git a/src/clutz.ts b/src/clutz.ts
@@ -21,15 +21,19 @@ import * as googmodule from './googmodule';
 import * as path from './path';
 import {isDeclaredInClutzDts} from './type_translator';
 
+interface ClutzHost {
+    /** See compiler_host.ts */
+    rootDirsRelative(fileName: string): string;
+}
+
 /**
  * Constructs a ts.CustomTransformerFactory that postprocesses the .d.ts
  * that are generated by ordinary TypeScript compilations to add some
  * Clutz-specific logic.  See generateClutzAliases.
  */
 export function makeDeclarationTransformerFactory(
     typeChecker: ts.TypeChecker,
-    googmoduleHost: googmodule.GoogModuleProcessorHost):
-    ts.CustomTransformerFactory {
+    host: ClutzHost&googmodule.GoogModuleProcessorHost): ts.CustomTransformerFactory {
   return (context: ts.TransformationContext): ts.CustomTransformer => {
     return {
       transformBundle(): ts.Bundle {
@@ -49,8 +53,7 @@ export function makeDeclarationTransformerFactory(
         //   import 'path/to/the/js_file';
         // so to for that import to resolve, you need to first import the clutz
         // d.ts that defines that declared module.
-        const imports =
-            gatherNecessaryClutzImports(googmoduleHost, typeChecker, file);
+        const imports = gatherNecessaryClutzImports(host, typeChecker, file);
         let importStmts: ts.Statement[]|undefined;
         if (imports.length > 0) {
           importStmts = imports.map(fileName => {
@@ -66,22 +69,56 @@ export function makeDeclarationTransformerFactory(
         // Construct `declare global {}` in the Clutz namespace for symbols
         // Clutz might use.
         const globalBlock = generateClutzAliases(
-            file, googmoduleHost.pathToModuleName('', file.fileName),
-            typeChecker, options);
+            file, host.pathToModuleName('', file.fileName), typeChecker,
+            options);
 
         // Only need to transform file if we needed one of the above additions.
         if (!importStmts && !globalBlock) return file;
 
-        return ts.factory.updateSourceFile(file, [
-          ...(importStmts ?? []),
-          ...file.statements,
-          ...(globalBlock ? [globalBlock] : []),
-        ]);
+        return ts.factory.updateSourceFile(
+            file,
+            ts.setTextRange(
+                ts.factory.createNodeArray([
+                  ...(importStmts ?? []),
+                  ...file.statements,
+                  ...(globalBlock ? [globalBlock] : []),
+                ]),
+                file.statements),
+            file.isDeclarationFile,
+            file.referencedFiles.map(
+                f => fixRelativeReference(f, file, options, host)),
+            // /// <reference types="x" /> directives are ignored under bazel.
+            /*typeReferences=*/[]);
       }
     };
   };
 }
 
+/**
+ * Fixes a relative reference from an output file with respect to multiple
+ * rootDirs. See https://github.com/Microsoft/TypeScript/issues/8245 for
+ * details.
+ */
+function fixRelativeReference(
+    reference: ts.FileReference, origin: ts.SourceFile,
+    options: ts.CompilerOptions, host: ClutzHost): ts.FileReference {
+  if (!options.outDir || !options.rootDir) {
+    return reference;
+  }
+  const originDir = path.dirname(origin.fileName);
+  // Where TypeScript expects the output to be.
+  const expectedOutDir =
+      path.join(options.outDir, path.relative(options.rootDir, originDir));
+  const referencedFile = path.join(expectedOutDir, reference.fileName);
+  // Where the output is actually emitted.
+  const actualOutDir =
+      path.join(options.outDir, host.rootDirsRelative(originDir));
+  const fixedReference = path.relative(actualOutDir, referencedFile);
+
+  reference.fileName = fixedReference;
+  return reference;
+}
+
 /** Compares two strings and returns a number suitable for use in sort(). */
 function stringCompare(a: string, b: string): number {
   if (a < b) return -1;

diff --git a/src/decorators.ts b/src/decorators.ts
@@ -98,7 +98,7 @@ export function transformDecoratorsOutputForClosurePropertyRenaming(diagnostics:
   return (context: ts.TransformationContext) => {
     const result: ts.Transformer<ts.SourceFile> = (sourceFile: ts.SourceFile) => {
       let nodeNeedingGoogReflect: undefined|ts.Node = undefined;
-      const visitor: ts.Visitor = (node) => {
+      const visitor = (node: ts.Node) => {
         const replacementNode = rewriteDecorator(node);
         if (replacementNode) {
           nodeNeedingGoogReflect = node;
@@ -107,9 +107,7 @@ export function transformDecoratorsOutputForClosurePropertyRenaming(diagnostics:
         return ts.visitEachChild(node, visitor, context);
       };
       let updatedSourceFile =
-          // TODO: go/ts50upgrade - Remove after upgrade.
-          // tslint:disable-next-line:no-unnecessary-type-assertion
-          ts.visitNode(sourceFile, visitor, ts.isSourceFile)!;
+          ts.visitNode(sourceFile, visitor, ts.isSourceFile);
       if (nodeNeedingGoogReflect !== undefined) {
         const statements = [...updatedSourceFile.statements];
         const googModuleIndex = statements.findIndex(isGoogModuleStatement);

diff --git a/src/enum_transformer.ts b/src/enum_transformer.ts
@@ -19,6 +19,7 @@
  * type resolve ("@type {Foo}").
  */
 
+import {TsickleHost} from 'tsickle';
 import * as ts from 'typescript';
 
 import * as jsdoc from './jsdoc';
@@ -95,7 +96,7 @@ export function getEnumType(typeChecker: ts.TypeChecker, enumDecl: ts.EnumDeclar
 /**
  * Transformer factory for the enum transformer. See fileoverview for details.
  */
-export function enumTransformer(typeChecker: ts.TypeChecker):
+export function enumTransformer(host: TsickleHost, typeChecker: ts.TypeChecker):
     (context: ts.TransformationContext) => ts.Transformer<ts.SourceFile> {
   return (context: ts.TransformationContext) => {
     function visitor<T extends ts.Node>(node: T): T|ts.Node[] {
@@ -180,7 +181,11 @@ export function enumTransformer(typeChecker: ts.TypeChecker):
                   /* modifiers */ undefined,
                   ts.factory.createVariableDeclarationList(
                       [varDecl],
-                      /* create a const var */ ts.NodeFlags.Const)),
+                      /* When using unoptimized namespaces, create a var
+                         declaration, otherwise create a const var. See b/157460535 */
+                      host.useDeclarationMergingTransformation ?
+                          ts.NodeFlags.Const :
+                          undefined)),
               node),
           node);
 

diff --git a/src/externs.ts b/src/externs.ts
@@ -295,24 +295,13 @@ export function generateExterns(
    *   interface Foo { x: number; }
    *   interface Foo { y: number; }
    * we only want to emit the "\@record" for Foo on the first one.
-   *
-   * The exception are variable declarations, which - in externs - do not assign a value:
-   *   /.. \@type {...} ./
-   *   var someVariable;
-   *   /.. \@type {...} ./
-   *   someNamespace.someVariable;
-   * If a later declaration wants to add additional properties on someVariable, tsickle must still
-   * emit an assignment into the object, as it's otherwise absent.
    */
   function isFirstValueDeclaration(decl: ts.DeclarationStatement): boolean {
     if (!decl.name) return true;
     const sym = typeChecker.getSymbolAtLocation(decl.name)!;
     if (!sym.declarations || sym.declarations.length < 2) return true;
     const earlierDecls = sym.declarations.slice(0, sym.declarations.indexOf(decl));
-    // Either there are no earlier declarations, or all of them are variables (see above). tsickle
-    // emits a value for all other declaration kinds (function for functions, classes, interfaces,
-    // {} object for namespaces).
-    return earlierDecls.length === 0 || earlierDecls.every(ts.isVariableDeclaration);
+    return earlierDecls.length === 0 || earlierDecls.every(d => ts.isVariableDeclaration(d) && d.getSourceFile() !== decl.getSourceFile());
   }
 
   /** Writes the actual variable statement of a Closure variable declaration. */