Repository: TypeScript
Issue: Typescript emits invalid AMD with custom tranformation of export syntax
Labels: Bug,Help Wanted

# 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

```ts
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.