Skip to content

Commit 6192c9b

Browse files
frigus02copybara-github
authored andcommitted
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
1 parent d5c2921 commit 6192c9b

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

55 files changed

+1303
-574
lines changed

demo/package.json

+1-1
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
"dependencies": {
99
"minimist": "^1.2.3",
1010
"tsickle": "file:../",
11-
"typescript": "5.2.2"
11+
"typescript": "5.3.2"
1212
},
1313
"devDependencies": {
1414
"@types/minimist": "1.2.0",

package.json

+2-2
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
"out/src/*"
1212
],
1313
"peerDependencies": {
14-
"typescript": "~5.1.5"
14+
"typescript": "~5.3.2"
1515
},
1616
"devDependencies": {
1717
"@types/diff-match-patch": "^1.0.32",
@@ -28,7 +28,7 @@
2828
"source-map-support": "^0.5.19",
2929
"tslib": "^2.2.0",
3030
"tslint": "^6.1.3",
31-
"typescript": "5.2.2"
31+
"typescript": "5.3.2"
3232
},
3333
"scripts": {
3434
"build": "tsc",

src/clutz.ts

+48-11
Original file line numberDiff line numberDiff line change
@@ -21,15 +21,19 @@ import * as googmodule from './googmodule';
2121
import * as path from './path';
2222
import {isDeclaredInClutzDts} from './type_translator';
2323

24+
interface ClutzHost {
25+
/** See compiler_host.ts */
26+
rootDirsRelative(fileName: string): string;
27+
}
28+
2429
/**
2530
* Constructs a ts.CustomTransformerFactory that postprocesses the .d.ts
2631
* that are generated by ordinary TypeScript compilations to add some
2732
* Clutz-specific logic. See generateClutzAliases.
2833
*/
2934
export function makeDeclarationTransformerFactory(
3035
typeChecker: ts.TypeChecker,
31-
googmoduleHost: googmodule.GoogModuleProcessorHost):
32-
ts.CustomTransformerFactory {
36+
host: ClutzHost&googmodule.GoogModuleProcessorHost): ts.CustomTransformerFactory {
3337
return (context: ts.TransformationContext): ts.CustomTransformer => {
3438
return {
3539
transformBundle(): ts.Bundle {
@@ -49,8 +53,7 @@ export function makeDeclarationTransformerFactory(
4953
// import 'path/to/the/js_file';
5054
// so to for that import to resolve, you need to first import the clutz
5155
// d.ts that defines that declared module.
52-
const imports =
53-
gatherNecessaryClutzImports(googmoduleHost, typeChecker, file);
56+
const imports = gatherNecessaryClutzImports(host, typeChecker, file);
5457
let importStmts: ts.Statement[]|undefined;
5558
if (imports.length > 0) {
5659
importStmts = imports.map(fileName => {
@@ -66,22 +69,56 @@ export function makeDeclarationTransformerFactory(
6669
// Construct `declare global {}` in the Clutz namespace for symbols
6770
// Clutz might use.
6871
const globalBlock = generateClutzAliases(
69-
file, googmoduleHost.pathToModuleName('', file.fileName),
70-
typeChecker, options);
72+
file, host.pathToModuleName('', file.fileName), typeChecker,
73+
options);
7174

7275
// Only need to transform file if we needed one of the above additions.
7376
if (!importStmts && !globalBlock) return file;
7477

75-
return ts.factory.updateSourceFile(file, [
76-
...(importStmts ?? []),
77-
...file.statements,
78-
...(globalBlock ? [globalBlock] : []),
79-
]);
78+
return ts.factory.updateSourceFile(
79+
file,
80+
ts.setTextRange(
81+
ts.factory.createNodeArray([
82+
...(importStmts ?? []),
83+
...file.statements,
84+
...(globalBlock ? [globalBlock] : []),
85+
]),
86+
file.statements),
87+
file.isDeclarationFile,
88+
file.referencedFiles.map(
89+
f => fixRelativeReference(f, file, options, host)),
90+
// /// <reference types="x" /> directives are ignored under bazel.
91+
/*typeReferences=*/[]);
8092
}
8193
};
8294
};
8395
}
8496

97+
/**
98+
* Fixes a relative reference from an output file with respect to multiple
99+
* rootDirs. See https://github.com/Microsoft/TypeScript/issues/8245 for
100+
* details.
101+
*/
102+
function fixRelativeReference(
103+
reference: ts.FileReference, origin: ts.SourceFile,
104+
options: ts.CompilerOptions, host: ClutzHost): ts.FileReference {
105+
if (!options.outDir || !options.rootDir) {
106+
return reference;
107+
}
108+
const originDir = path.dirname(origin.fileName);
109+
// Where TypeScript expects the output to be.
110+
const expectedOutDir =
111+
path.join(options.outDir, path.relative(options.rootDir, originDir));
112+
const referencedFile = path.join(expectedOutDir, reference.fileName);
113+
// Where the output is actually emitted.
114+
const actualOutDir =
115+
path.join(options.outDir, host.rootDirsRelative(originDir));
116+
const fixedReference = path.relative(actualOutDir, referencedFile);
117+
118+
reference.fileName = fixedReference;
119+
return reference;
120+
}
121+
85122
/** Compares two strings and returns a number suitable for use in sort(). */
86123
function stringCompare(a: string, b: string): number {
87124
if (a < b) return -1;

src/decorators.ts

+2-4
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ export function transformDecoratorsOutputForClosurePropertyRenaming(diagnostics:
9898
return (context: ts.TransformationContext) => {
9999
const result: ts.Transformer<ts.SourceFile> = (sourceFile: ts.SourceFile) => {
100100
let nodeNeedingGoogReflect: undefined|ts.Node = undefined;
101-
const visitor: ts.Visitor = (node) => {
101+
const visitor = (node: ts.Node) => {
102102
const replacementNode = rewriteDecorator(node);
103103
if (replacementNode) {
104104
nodeNeedingGoogReflect = node;
@@ -107,9 +107,7 @@ export function transformDecoratorsOutputForClosurePropertyRenaming(diagnostics:
107107
return ts.visitEachChild(node, visitor, context);
108108
};
109109
let updatedSourceFile =
110-
// TODO: go/ts50upgrade - Remove after upgrade.
111-
// tslint:disable-next-line:no-unnecessary-type-assertion
112-
ts.visitNode(sourceFile, visitor, ts.isSourceFile)!;
110+
ts.visitNode(sourceFile, visitor, ts.isSourceFile);
113111
if (nodeNeedingGoogReflect !== undefined) {
114112
const statements = [...updatedSourceFile.statements];
115113
const googModuleIndex = statements.findIndex(isGoogModuleStatement);

src/enum_transformer.ts

+7-2
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
* type resolve ("@type {Foo}").
2020
*/
2121

22+
import {TsickleHost} from 'tsickle';
2223
import * as ts from 'typescript';
2324

2425
import * as jsdoc from './jsdoc';
@@ -95,7 +96,7 @@ export function getEnumType(typeChecker: ts.TypeChecker, enumDecl: ts.EnumDeclar
9596
/**
9697
* Transformer factory for the enum transformer. See fileoverview for details.
9798
*/
98-
export function enumTransformer(typeChecker: ts.TypeChecker):
99+
export function enumTransformer(host: TsickleHost, typeChecker: ts.TypeChecker):
99100
(context: ts.TransformationContext) => ts.Transformer<ts.SourceFile> {
100101
return (context: ts.TransformationContext) => {
101102
function visitor<T extends ts.Node>(node: T): T|ts.Node[] {
@@ -180,7 +181,11 @@ export function enumTransformer(typeChecker: ts.TypeChecker):
180181
/* modifiers */ undefined,
181182
ts.factory.createVariableDeclarationList(
182183
[varDecl],
183-
/* create a const var */ ts.NodeFlags.Const)),
184+
/* When using unoptimized namespaces, create a var
185+
declaration, otherwise create a const var. See b/157460535 */
186+
host.useDeclarationMergingTransformation ?
187+
ts.NodeFlags.Const :
188+
undefined)),
184189
node),
185190
node);
186191

src/externs.ts

+1-12
Original file line numberDiff line numberDiff line change
@@ -295,24 +295,13 @@ export function generateExterns(
295295
* interface Foo { x: number; }
296296
* interface Foo { y: number; }
297297
* we only want to emit the "\@record" for Foo on the first one.
298-
*
299-
* The exception are variable declarations, which - in externs - do not assign a value:
300-
* /.. \@type {...} ./
301-
* var someVariable;
302-
* /.. \@type {...} ./
303-
* someNamespace.someVariable;
304-
* If a later declaration wants to add additional properties on someVariable, tsickle must still
305-
* emit an assignment into the object, as it's otherwise absent.
306298
*/
307299
function isFirstValueDeclaration(decl: ts.DeclarationStatement): boolean {
308300
if (!decl.name) return true;
309301
const sym = typeChecker.getSymbolAtLocation(decl.name)!;
310302
if (!sym.declarations || sym.declarations.length < 2) return true;
311303
const earlierDecls = sym.declarations.slice(0, sym.declarations.indexOf(decl));
312-
// Either there are no earlier declarations, or all of them are variables (see above). tsickle
313-
// emits a value for all other declaration kinds (function for functions, classes, interfaces,
314-
// {} object for namespaces).
315-
return earlierDecls.length === 0 || earlierDecls.every(ts.isVariableDeclaration);
304+
return earlierDecls.length === 0 || earlierDecls.every(d => ts.isVariableDeclaration(d) && d.getSourceFile() !== decl.getSourceFile());
316305
}
317306

318307
/** Writes the actual variable statement of a Closure variable declaration. */

0 commit comments

Comments
 (0)