Typescript emits invalid AMD with custom tranformation of export syntax
#53,925 opened on Apr 20, 2023
Repository metrics
- Stars
- (48,455 stars)
- PR merge metrics
- (Avg merge 6d 17h) (9 merged PRs in 30d)
Description
Bug Report
🔎 Search Terms
exports transformer transformation emit AMD
FYI this is the result of my attempt at a workaround for #52539 given that there's been no interest in fixing that in Typescript itself.
🕗 Version & Regression Information
This is the behavior in every version I tried. In particular I tried with Typescript 4.7.4 (my current target... my codebase is behind) and Typescript 5.0.4 (latest stable).
⏯ Playground Link
As this bug relies on a transformer, it cannot be reproduced in the playground.
💻 Code
My goal was to write a transformer that took input like export let foo = ... and turned it into let foo = ...; export {foo} that I could use as a before transformer to transpileModule; based on my analysis of typescript's emit, this should be a workable workaround for #52539... if not for this bug. I've left a bunch of my working tests below to illustrate that my transform behaves as expected, EXCEPT when used with AMD. In all fairness I didn't test with CommonJS or System - just ESNext and AMD as the former seemed easier to test with and the latter is my target application.
I'm running this with jest 28.1.0, though this doesn't make use of any interesting jest features so probably could run under a different version or even another test runner with minimal tweaks.
This test is written for TS 4.7.4 but with the one commented line below removed also works in 5.0.4
import ts from 'typescript';
/* note for maintainers: in future typescripts the number of params used in some
* of the typescript helper functions may change. Consult node_modules/typescript/lib/typescript.d.ts
* to see what your current version of typescript expects. By writing in typescript, these should
* be type errors though some of these apis are permissive about allowing undefined
*/
function getIdentifierNamesFromBindingPatterns(
name: ts.ArrayBindingPattern | ts.ObjectBindingPattern | ts.Identifier
): string[] {
if (ts.isIdentifier(name)) {
return [name.text];
}
const bindingElements: ts.BindingElement[] = [];
for (const n of name.elements) {
if (!ts.isOmittedExpression(n)) {
bindingElements.push(n);
}
}
return bindingElements
.map((bindingElement: ts.BindingElement) => bindingElement.name)
.flatMap((bindingName: ts.BindingName) => getIdentifierNamesFromBindingPatterns(bindingName));
}
const exportTransformer: ts.TransformerFactory<ts.SourceFile> = (context) => {
const newExports: ts.ExportDeclaration[] = [];
function visitor(node: ts.Node): ts.Node {
// match any `export let` or `export const`
if (ts.isVariableStatement(node)) {
const vNode = node as ts.VariableStatement;
if (
vNode.modifiers?.some((m) => m.kind == ts.SyntaxKind.ExportKeyword) &&
vNode.declarationList.flags & (ts.NodeFlags.Const | ts.NodeFlags.Let)
) {
// and transform them to let foo = ...; export {foo};
// we can skip visiting the children because we know we won't need to
// change the children - export statements are only valid at the top level.
// we can't easily add new export statements here, so construct them and throw
// them into newExports for us to add to the SourceFile
newExports.push(
ts.factory.createExportDeclaration(
[], // decorators - DELETE THIS LINE FOR TYPESCRIPT 5.0
[], // modifiers
false, // isTypeOnly
ts.factory.createNamedExports(
vNode.declarationList.declarations
.flatMap((d) => getIdentifierNamesFromBindingPatterns(d.name))
.map((identName: string) =>
ts.factory.createExportSpecifier(false, undefined, identName)
)
), // exportClause
undefined, // moduleSpecifier
undefined // assertClause
)
);
// strip out the export modifier on existing export let & export const
return ts.factory.updateVariableStatement(
node,
node.modifiers?.filter((m) => m.kind != ts.SyntaxKind.ExportKeyword),
node.declarationList
);
}
}
return ts.visitEachChild(node, visitor, context);
}
return function (sourceFile: ts.SourceFile): ts.SourceFile {
sourceFile = ts.visitNode(sourceFile, visitor);
return ts.factory.updateSourceFile(sourceFile, [...sourceFile.statements, ...newExports]);
};
};
/* test code begins here. These can be in the same file for the sake of simplifying the reproduction
* but when used in real code would probably be separate
*/
function transpileWithTransform(
input: string,
module: typescript.ModuleKind = typescript.ModuleKind.ESNext
): string {
const output = typescript.transpileModule(input, {
compilerOptions: {
module: module,
target: typescript.ScriptTarget.ES2017,
},
reportDiagnostics: true,
transformers: {
before: [exportTransformer],
},
});
if (output.diagnostics && output.diagnostics.length) {
let msg = 'Failed to compile:\n';
output.diagnostics.forEach(function (diagnostic) {
let line, character;
if (typeof diagnostic.start === 'number') {
const lineStarts = (typescript as any).computeLineStarts(input);
const pos = (typescript as any).computeLineAndCharacterOfPosition(
lineStarts,
diagnostic.start
);
line = pos.line;
character = pos.character;
} else {
line = 0;
character = 0;
}
let message = typescript.flattenDiagnosticMessageText(diagnostic.messageText, '\n');
msg += `${line + 1},${character + 1}: ${message}\n`;
});
throw new Error(msg);
}
return output.outputText;
}
describe('ts-transform-export-syntax', () => {
it('transforms export let and export const', () => {
expect(transpileWithTransform(`export let foo = "foo";`)).toEqual(
`let foo = "foo";\nexport { foo };\n`
);
expect(transpileWithTransform(`export const foo = "foo";`)).toEqual(
`const foo = "foo";\nexport { foo };\n`
);
});
it('transforms export let & export const with multiple variable declarations in one statement', () => {
expect(transpileWithTransform(`export let foo = "foo", bar;`)).toEqual(
`let foo = "foo", bar;\nexport { foo, bar };\n`
);
expect(transpileWithTransform(`export const foo = "foo", bar = "bar";`)).toEqual(
`const foo = "foo", bar = "bar";\nexport { foo, bar };\n`
);
});
it('transforms array destructuring export', () => {
expect(transpileWithTransform(`export let [foo, bar] = [1, 2];`)).toEqual(
`let [foo, bar] = [1, 2];\nexport { foo, bar };\n`
);
});
it('transforms object destructuring exports', () => {
expect(transpileWithTransform(`export const {name1, name2: bar} = o;`)).toEqual(
`const { name1, name2: bar } = o;\nexport { name1, bar };\n`
);
});
it('handles ridiculous arbitrary destructuring nesting', () => {
expect(
transpileWithTransform(`export let [a, b, {c, d: e, f: [g, h, {i}, ...rest2]}, ...rest] = o;`)
).toEqual(
`let [a, b, { c, d: e, f: [g, h, { i }, ...rest2] }, ...rest] = o;\nexport { a, b, c, e, g, h, i, rest2, rest };\n`
);
expect(
transpileWithTransform(`
export const [a, b, ...{ pop, push }] = array;
export const [c, d, ...[e, f]] = array;
`)
).toEqual(`const [a, b, ...{ pop, push }] = array;
const [c, d, ...[e, f]] = array;
export { a, b, pop, push };
export { c, d, e, f };
`);
});
it("doesn't transform class and function exports", () => {
expect(transpileWithTransform(`export function functionName() {};`)).toEqual(
`export function functionName() { }\n;\n`
);
expect(transpileWithTransform(`export class ClassName {}`)).toEqual(
`export class ClassName {\n}\n`
);
});
it('works with AMD output', () => {
const codeToTransform = `export let foo = 1; export const bar = 2; export function baz() {foo = 3;}`;
const expectedTransformedESNext = `let foo = 1;
const bar = 2;
export function baz() { foo = 3; }
export { foo };
export { bar };
`;
const expectedTransformedAMD = `define(["require", "exports"], function (require, exports) {
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.bar = exports.foo = exports.baz = void 0;
let foo = 1;
exports.foo = foo;
const bar = 2;
exports.bar = bar;
function baz() { exports.foo = foo = 3; }
exports.baz = baz;
});
`;
// first verify the transform behaves as expected on ESNext output
expect(transpileWithTransform(codeToTransform, typescript.ModuleKind.ESNext)).toEqual(
expectedTransformedESNext
);
// control: if the input is pre-transformed so the transform has nothing to do
// we should get the expected AMD output
expect(transpileWithTransform(expectedTransformedESNext, typescript.ModuleKind.AMD)).toEqual(
expectedTransformedAMD
);
// if we run the transform and compile to AMD, we should get the same AMD result
expect(transpileWithTransform(codeToTransform, typescript.ModuleKind.AMD)).toEqual(
expectedTransformedAMD
);
});
});
🙁 Actual behavior
the last test fails with some clearly nonsense generated output - exports.foo = exports.foo is clear nonsense.
● ts-transform-export-syntax › works with AMD output
expect(received).toEqual(expected) // deep equality
- Expected - 3
+ Received + 3
define(["require", "exports"], function (require, exports) {
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
exports.bar = exports.foo = exports.baz = void 0;
let foo = 1;
- exports.foo = foo;
+ exports.foo = exports.foo;
const bar = 2;
- exports.bar = bar;
+ exports.bar = exports.bar;
- function baz() { exports.foo = foo = 3; }
+ function baz() { exports.foo = 3; }
exports.baz = baz;
});
↵
183 | );
184 | // if we run the transform and compile to AMD, we should get the same AMD result
> 185 | expect(transpileWithTransform(codeToTransform, typescript.ModuleKind.AMD)).toEqual(
| ^
186 | expectedTransformedAMD
187 | );
188 | });
However the earlier expect()s in the same test pass - showing that we get a different result on the transformed input vs if we let the transformer run on the original input during the same transpileModule.
I'm fairly certain (~90%) that the bug is with typescript, and not with the transform itself. I spent a lot of time stepping through the code today in a debugger and it appears that everything is fine until substituteExpressionIdentifier decides to replace some of the foo and bar references with exports.foo and exports.bar respectively https://github.com/microsoft/TypeScript/blob/ac55b297b755131af0388327e8115467c982c04b/src/compiler/transformers/module/module.ts#L2096-L2097 - this branch doesn't seem to be taken with the expectedTransformedESNext as input; only the final transpileWithTransform call of this test case reaches https://github.com/microsoft/TypeScript/blob/ac55b297b755131af0388327e8115467c982c04b/src/compiler/transformers/module/module.ts#L2098.
🙂 Expected behavior
I expect all tests to pass. I definitely expect that doing a transform by hand before passing the code to typescript, vs transform the AST with a before transformer, will result in the same behavior and it's incredibly weird here that they don't.