Skip to content

Commit 7075434

Browse files
tsjs-language-engcopybara-github
tsjs-language-eng
authored andcommitted
Properly transpile array destructuring pattern in the highest level.
Before, it was missing a variable declaration after traspilation for identifiers used in alias assignment expressions. The cause was missing exports on actual identifiers that needed import, and ended up getting removed by the passes coming aftewards, because jsdoc_transformer was renaming identifiers destructuring statements, but not the usage of those identifiers properly because exported symbol ends up being used as `exports.identifier_destructured`, but it was being used as `identifier_destructured` pattern. PiperOrigin-RevId: 597281499
1 parent d5c2921 commit 7075434

Some content is hidden

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

52 files changed

+1243
-562
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)