Skip to content

Commit

Permalink
Properly transpile exported array destructuring.
Browse files Browse the repository at this point in the history
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
  • Loading branch information
tsjs-language-eng authored and copybara-github committed Jan 12, 2024
1 parent d5c2921 commit 8c58e41
Show file tree
Hide file tree
Showing 52 changed files with 1,242 additions and 563 deletions.
2 changes: 1 addition & 1 deletion demo/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"out/src/*"
],
"peerDependencies": {
"typescript": "~5.1.5"
"typescript": "~5.3.2"
},
"devDependencies": {
"@types/diff-match-patch": "^1.0.32",
Expand All @@ -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",
Expand Down
59 changes: 48 additions & 11 deletions src/clutz.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 => {
Expand All @@ -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;
Expand Down
6 changes: 2 additions & 4 deletions src/decorators.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand Down
9 changes: 7 additions & 2 deletions src/enum_transformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
* type resolve ("@type {Foo}").
*/

import {TsickleHost} from 'tsickle';
import * as ts from 'typescript';

import * as jsdoc from './jsdoc';
Expand Down Expand Up @@ -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[] {
Expand Down Expand Up @@ -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);

Expand Down
13 changes: 1 addition & 12 deletions src/externs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down
Loading

0 comments on commit 8c58e41

Please sign in to comment.