Skip to content

Commit 0d8deb0

Browse files
alxhubkara
authored andcommitted
fix(compiler-cli): generate proper exports.* identifiers in cjs output (#22564)
When the compiler generates a reference to an exported variable in the same file, it inserts a synthetic ts.Identifier node. In CommonJS output, this synthetic node would not be properly rewritten with an `exports.` prefix. This change sets the TS original node property on the synthetic node we generate, which ensures TS knows to rewrite it in CommonJS output. PR Close #22564
1 parent e8326e6 commit 0d8deb0

File tree

2 files changed

+65
-5
lines changed

2 files changed

+65
-5
lines changed

packages/compiler-cli/src/transformers/node_emitter.ts

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ export function updateSourceFile(
6363
sourceFile: ts.SourceFile, module: PartialModule,
6464
context: ts.TransformationContext): [ts.SourceFile, Map<ts.Node, Node>] {
6565
const converter = new _NodeEmitterVisitor();
66+
converter.loadExportedVariableIdentifiers(sourceFile);
67+
6668
const prefixStatements = module.statements.filter(statement => !(statement instanceof ClassStmt));
6769
const classes =
6870
module.statements.filter(statement => statement instanceof ClassStmt) as ClassStmt[];
@@ -158,6 +160,11 @@ function createLiteral(value: any) {
158160
}
159161
}
160162

163+
function isExportTypeStatement(statement: ts.Statement): boolean {
164+
return !!statement.modifiers &&
165+
statement.modifiers.some(mod => mod.kind === ts.SyntaxKind.ExportKeyword);
166+
}
167+
161168
/**
162169
* Visits an output ast and produces the corresponding TypeScript synthetic nodes.
163170
*/
@@ -166,6 +173,26 @@ class _NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor {
166173
private _importsWithPrefixes = new Map<string, string>();
167174
private _reexports = new Map<string, {name: string, as: string}[]>();
168175
private _templateSources = new Map<ParseSourceFile, ts.SourceMapSource>();
176+
private _exportedVariableIdentifiers = new Map<string, ts.Identifier>();
177+
178+
/**
179+
* Process the source file and collect exported identifiers that refer to variables.
180+
*
181+
* Only variables are collected because exported classes still exist in the module scope in
182+
* CommonJS, whereas variables have their declarations moved onto the `exports` object, and all
183+
* references are updated accordingly.
184+
*/
185+
loadExportedVariableIdentifiers(sourceFile: ts.SourceFile): void {
186+
sourceFile.statements.forEach(statement => {
187+
if (ts.isVariableStatement(statement) && isExportTypeStatement(statement)) {
188+
statement.declarationList.declarations.forEach(declaration => {
189+
if (ts.isIdentifier(declaration.name)) {
190+
this._exportedVariableIdentifiers.set(declaration.name.text, declaration.name);
191+
}
192+
});
193+
}
194+
});
195+
}
169196

170197
getReexports(): ts.Statement[] {
171198
return Array.from(this._reexports.entries())
@@ -612,7 +639,8 @@ class _NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor {
612639
}
613640

614641
private _visitIdentifier(value: ExternalReference): ts.Expression {
615-
const {name, moduleName} = value;
642+
// name can only be null during JIT which never executes this code.
643+
const moduleName = value.moduleName, name = value.name !;
616644
let prefixIdent: ts.Identifier|null = null;
617645
if (moduleName) {
618646
let prefix = this._importsWithPrefixes.get(moduleName);
@@ -622,10 +650,17 @@ class _NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor {
622650
}
623651
prefixIdent = ts.createIdentifier(prefix);
624652
}
625-
// name can only be null during JIT which never executes this code.
626-
let result: ts.Expression =
627-
prefixIdent ? ts.createPropertyAccess(prefixIdent, name !) : ts.createIdentifier(name !);
628-
return result;
653+
if (prefixIdent) {
654+
return ts.createPropertyAccess(prefixIdent, name);
655+
} else {
656+
const id = ts.createIdentifier(name);
657+
if (this._exportedVariableIdentifiers.has(name)) {
658+
// In order for this new identifier node to be properly rewritten in CommonJS output,
659+
// it must have its original node set to a parsed instance of the same identifier.
660+
ts.setOriginalNode(id, this._exportedVariableIdentifiers.get(name));
661+
}
662+
return id;
663+
}
629664
}
630665
}
631666

packages/compiler-cli/test/ngc_spec.ts

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2158,5 +2158,30 @@ describe('ngc transformer command-line', () => {
21582158
expect(source).toMatch(/ngInjectableDef.*token: Service/);
21592159
expect(source).toMatch(/ngInjectableDef.*scope: .+\.Module/);
21602160
});
2161+
2162+
it('generates exports.* references when outputting commonjs', () => {
2163+
writeConfig(`{
2164+
"extends": "./tsconfig-base.json",
2165+
"compilerOptions": {
2166+
"module": "commonjs"
2167+
},
2168+
"files": ["service.ts"]
2169+
}`);
2170+
const source = compileService(`
2171+
import {Inject, Injectable, InjectionToken, APP_ROOT_SCOPE} from '@angular/core';
2172+
import {Module} from './module';
2173+
2174+
export const TOKEN = new InjectionToken<string>('test token', {
2175+
scope: APP_ROOT_SCOPE,
2176+
factory: () => 'this is a test',
2177+
});
2178+
2179+
@Injectable({scope: APP_ROOT_SCOPE})
2180+
export class Service {
2181+
constructor(@Inject(TOKEN) token: any) {}
2182+
}
2183+
`);
2184+
expect(source).toMatch(/new Service\(i0\.inject\(exports\.TOKEN\)\);/);
2185+
});
21612186
});
21622187
});

0 commit comments

Comments
 (0)