Skip to content

Commit

Permalink
fix(compiler-cli): generate proper exports.* identifiers in cjs output
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
alxhub committed Mar 2, 2018
1 parent 0451fd9 commit ca6faa7
Show file tree
Hide file tree
Showing 2 changed files with 56 additions and 3 deletions.
34 changes: 31 additions & 3 deletions packages/compiler-cli/src/transformers/node_emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ export function updateSourceFile(
sourceFile: ts.SourceFile, module: PartialModule,
context: ts.TransformationContext): [ts.SourceFile, Map<ts.Node, Node>] {
const converter = new _NodeEmitterVisitor();
converter.loadExportedVariableIdentifiers(sourceFile);

const prefixStatements = module.statements.filter(statement => !(statement instanceof ClassStmt));
const classes =
module.statements.filter(statement => statement instanceof ClassStmt) as ClassStmt[];
Expand Down Expand Up @@ -158,6 +160,11 @@ function createLiteral(value: any) {
}
}

function isExportTypeStatement(statement: ts.Statement): boolean {
return !!statement.modifiers &&
statement.modifiers.some(mod => mod.kind === ts.SyntaxKind.ExportKeyword);
}

/**
* Visits an output ast and produces the corresponding TypeScript synthetic nodes.
*/
Expand All @@ -166,6 +173,19 @@ class _NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor {
private _importsWithPrefixes = new Map<string, string>();
private _reexports = new Map<string, {name: string, as: string}[]>();
private _templateSources = new Map<ParseSourceFile, ts.SourceMapSource>();
private _exportedVariableIdentifiers = new Map<string, ts.Identifier>();

loadExportedVariableIdentifiers(sourceFile: ts.SourceFile): void {
sourceFile.statements.forEach(statement => {
if (ts.isVariableStatement(statement) && isExportTypeStatement(statement)) {
statement.declarationList.declarations.forEach(declaration => {
if (ts.isIdentifier(declaration.name)) {
this._exportedVariableIdentifiers.set(declaration.name.text, declaration.name);
}
});
}
});
}

getReexports(): ts.Statement[] {
return Array.from(this._reexports.entries())
Expand Down Expand Up @@ -623,9 +643,17 @@ class _NodeEmitterVisitor implements StatementVisitor, ExpressionVisitor {
prefixIdent = ts.createIdentifier(prefix);
}
// name can only be null during JIT which never executes this code.
let result: ts.Expression =
prefixIdent ? ts.createPropertyAccess(prefixIdent, name !) : ts.createIdentifier(name !);
return result;
if (prefixIdent) {
return ts.createPropertyAccess(prefixIdent, name !);
} else {
const id = ts.createIdentifier(name !);
if (this._exportedVariableIdentifiers.has(name !)) {
// In order for this new identifier node to be properly rewritten in CommonJS output,
// it must have its original node set to a parsed instance of the same identifier.
ts.setOriginalNode(id, this._exportedVariableIdentifiers.get(name !));
}
return id;
}
}
}

Expand Down
25 changes: 25 additions & 0 deletions packages/compiler-cli/test/ngc_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2113,5 +2113,30 @@ describe('ngc transformer command-line', () => {
expect(source).toMatch(/ngInjectableDef.*token: Service/);
expect(source).toMatch(/ngInjectableDef.*scope: .+\.Module/);
});

it('generates exports.* references when outputting commonjs', () => {
writeConfig(`{
"extends": "./tsconfig-base.json",
"compilerOptions": {
"module": "commonjs"
},
"files": ["service.ts"]
}`);
const source = compileService(`
import {Inject, Injectable, InjectionToken, APP_ROOT_SCOPE} from '@angular/core';
import {Module} from './module';
export const TOKEN = new InjectionToken<string>('test token', {
scope: APP_ROOT_SCOPE,
factory: () => 'this is a test',
});
@Injectable({scope: APP_ROOT_SCOPE})
export class Service {
constructor(@Inject(TOKEN) token: any) {}
}
`);
expect(source).toMatch(/new Service\(i0\.inject\(exports\.TOKEN\)\);/)
});
});
});

0 comments on commit ca6faa7

Please sign in to comment.