fix(compiler): reexport less symbols in .ngfactory.ts files (#19884)

* don't reexport symbols that the user already reexported
* never reexport symbols that are part of arguments of non simple function calls

Fixes #19883

PR Close #19884
This commit is contained in:
Tobias Bosch 2017-10-23 17:51:19 -07:00 committed by Matias Niemelä
parent 04eb80cc2b
commit 420852e2f5
7 changed files with 284 additions and 65 deletions

View File

@ -6,7 +6,9 @@
* found in the LICENSE file at https://angular.io/license
*/
import {createLoweredSymbol, isLoweredSymbol} from '@angular/compiler';
import * as ts from 'typescript';
import {CollectorOptions, MetadataCollector, MetadataValue, ModuleMetadata, isMetadataGlobalReferenceExpression} from '../metadata/index';
export interface LoweringRequest {
@ -225,14 +227,12 @@ function shouldLower(node: ts.Node | undefined): boolean {
return true;
}
const REWRITE_PREFIX = '\u0275';
function isPrimitive(value: any): boolean {
return Object(value) !== value;
}
function isRewritten(value: any): boolean {
return isMetadataGlobalReferenceExpression(value) && value.name.startsWith(REWRITE_PREFIX);
return isMetadataGlobalReferenceExpression(value) && isLoweredSymbol(value.name);
}
function isLiteralFieldNamed(node: ts.Node, names: Set<string>): boolean {
@ -276,7 +276,7 @@ export class LowerMetadataCache implements RequestsMap {
private getMetadataAndRequests(sourceFile: ts.SourceFile): MetadataAndLoweringRequests {
let identNumber = 0;
const freshIdent = () => REWRITE_PREFIX + identNumber++;
const freshIdent = () => createLoweredSymbol(identNumber++);
const requests = new Map<number, LoweringRequest>();
const isExportedSymbol = (() => {

View File

@ -10,7 +10,7 @@ import {Summary, SummaryResolver} from '../summary_resolver';
import {StaticSymbol, StaticSymbolCache} from './static_symbol';
import {deserializeSummaries} from './summary_serializer';
import {ngfactoryFilePath, stripGeneratedFileSuffix, summaryFileName} from './util';
import {stripGeneratedFileSuffix, summaryFileName} from './util';
export interface AotSummaryResolverHost {
/**
@ -119,11 +119,7 @@ export class AotSummaryResolver implements SummaryResolver<StaticSymbol> {
if (moduleName) {
this.knownFileNameToModuleNames.set(filePath, moduleName);
}
importAs.forEach((importAs) => {
this.importAs.set(
importAs.symbol,
this.staticSymbolCache.get(ngfactoryFilePath(filePath), importAs.importAs));
});
importAs.forEach((importAs) => { this.importAs.set(importAs.symbol, importAs.importAs); });
}
return hasSummary;
}

View File

@ -12,7 +12,7 @@ import {OutputContext, ValueTransformer, ValueVisitor, visitValue} from '../util
import {StaticSymbol, StaticSymbolCache} from './static_symbol';
import {ResolvedStaticSymbol, StaticSymbolResolver} from './static_symbol_resolver';
import {summaryForJitFileName, summaryForJitName} from './util';
import {isLoweredSymbol, ngfactoryFilePath, summaryForJitFileName, summaryForJitName} from './util';
export function serializeSummaries(
srcFileName: string, forJitCtx: OutputContext | null,
@ -38,7 +38,7 @@ export function serializeSummaries(
});
const {json, exportAs} = toJsonSerializer.serialize();
if (forJitCtx) {
const forJitSerializer = new ForJitSerializer(forJitCtx, symbolResolver);
const forJitSerializer = new ForJitSerializer(forJitCtx, symbolResolver, summaryResolver);
types.forEach(({summary, metadata}) => { forJitSerializer.addSourceType(summary, metadata); });
toJsonSerializer.unprocessedSymbolSummariesBySymbol.forEach((summary) => {
if (summaryResolver.isLibraryFile(summary.symbol.filePath) && summary.type) {
@ -55,7 +55,7 @@ export function deserializeSummaries(
libraryFileName: string, json: string): {
moduleName: string | null,
summaries: Summary<StaticSymbol>[],
importAs: {symbol: StaticSymbol, importAs: string}[]
importAs: {symbol: StaticSymbol, importAs: StaticSymbol}[]
} {
const deserializer = new FromJsonDeserializer(symbolCache, summaryResolver);
return deserializer.deserialize(libraryFileName, json);
@ -83,6 +83,7 @@ class ToJsonSerializer extends ValueTransformer {
// Note: This only contains symbols without members.
private symbols: StaticSymbol[] = [];
private indexBySymbol = new Map<StaticSymbol, number>();
private reexportedBy = new Map<StaticSymbol, StaticSymbol>();
// This now contains a `__symbol: number` in the place of
// StaticSymbols, but otherwise has the same shape as the original objects.
private processedSummaryBySymbol = new Map<StaticSymbol, any>();
@ -126,9 +127,32 @@ class ToJsonSerializer extends ValueTransformer {
}
});
metadata = clone;
} else if (isCall(metadata)) {
if (!isFunctionCall(metadata) && !isMethodCallOnVariable(metadata)) {
// Don't store complex calls as we won't be able to simplify them anyways later on.
metadata = {
__symbolic: 'error',
message: 'Complex function calls are not supported.',
};
}
}
// Note: We need to keep storing ctor calls for e.g.
// `export const x = new InjectionToken(...)`
unprocessedSummary.metadata = metadata;
processedSummary.metadata = this.processValue(metadata, SerializationFlags.ResolveValue);
if (metadata instanceof StaticSymbol &&
this.summaryResolver.isLibraryFile(metadata.filePath)) {
const declarationSymbol = this.symbols[this.indexBySymbol.get(metadata) !];
if (!isLoweredSymbol(declarationSymbol.name)) {
// Note: symbols that were introduced during codegen in the user file can have a reexport
// if a user used `export *`. However, we can't rely on this as tsickle will change
// `export *` into named exports, using only the information from the typechecker.
// As we introduce the new symbols after typecheck, Tsickle does not know about them,
// and omits them when expanding `export *`.
// So we have to keep reexporting these symbols manually via .ngfactory files.
this.reexportedBy.set(declarationSymbol, summary.symbol);
}
}
}
if (!unprocessedSummary.type && summary.type) {
unprocessedSummary.type = summary.type;
@ -161,12 +185,17 @@ class ToJsonSerializer extends ValueTransformer {
summaries: this.processedSummaries,
symbols: this.symbols.map((symbol, index) => {
symbol.assertNoMembers();
let importAs: string = undefined !;
let importAs: string|number = undefined !;
if (this.summaryResolver.isLibraryFile(symbol.filePath)) {
const summary = this.unprocessedSymbolSummariesBySymbol.get(symbol);
if (!summary || !summary.metadata || summary.metadata.__symbolic !== 'interface') {
importAs = `${symbol.name}_${index}`;
exportAs.push({symbol, exportAs: importAs});
const reexportSymbol = this.reexportedBy.get(symbol);
if (reexportSymbol) {
importAs = this.indexBySymbol.get(reexportSymbol) !;
} else {
const summary = this.unprocessedSymbolSummariesBySymbol.get(symbol);
if (!summary || !summary.metadata || summary.metadata.__symbolic !== 'interface') {
importAs = `${symbol.name}_${index}`;
exportAs.push({symbol, exportAs: importAs});
}
}
}
return {
@ -246,29 +275,35 @@ class ToJsonSerializer extends ValueTransformer {
}
class ForJitSerializer {
private data = new Map<StaticSymbol, {
private data: Array<{
summary: CompileTypeSummary,
metadata: CompileNgModuleMetadata|CompileDirectiveMetadata|CompilePipeMetadata|
CompileTypeMetadata|null,
isLibrary: boolean
}>();
}> = [];
constructor(private outputCtx: OutputContext, private symbolResolver: StaticSymbolResolver) {}
constructor(
private outputCtx: OutputContext, private symbolResolver: StaticSymbolResolver,
private summaryResolver: SummaryResolver<StaticSymbol>) {}
addSourceType(
summary: CompileTypeSummary, metadata: CompileNgModuleMetadata|CompileDirectiveMetadata|
CompilePipeMetadata|CompileTypeMetadata) {
this.data.set(summary.type.reference, {summary, metadata, isLibrary: false});
this.data.push({summary, metadata, isLibrary: false});
}
addLibType(summary: CompileTypeSummary) {
this.data.set(summary.type.reference, {summary, metadata: null, isLibrary: true});
this.data.push({summary, metadata: null, isLibrary: true});
}
serialize(exportAs: {symbol: StaticSymbol, exportAs: string}[]): void {
serialize(exportAsArr: {symbol: StaticSymbol, exportAs: string}[]): void {
const exportAsBySymbol = new Map<StaticSymbol, string>();
for (const {symbol, exportAs} of exportAsArr) {
exportAsBySymbol.set(symbol, exportAs);
}
const ngModuleSymbols = new Set<StaticSymbol>();
Array.from(this.data.values()).forEach(({summary, metadata, isLibrary}) => {
for (const {summary, metadata, isLibrary} of this.data) {
if (summary.summaryKind === CompileSummaryKind.NgModule) {
// collect the symbols that refer to NgModule classes.
// Note: we can't just rely on `summary.type.summaryKind` to determine this as
@ -276,7 +311,9 @@ class ForJitSerializer {
// See serializeSummaries for details.
ngModuleSymbols.add(summary.type.reference);
const modSummary = <CompileNgModuleSummary>summary;
modSummary.modules.forEach((mod) => { ngModuleSymbols.add(mod.reference); });
for (const mod of modSummary.modules) {
ngModuleSymbols.add(mod.reference);
}
}
if (!isLibrary) {
const fnName = summaryForJitName(summary.type.reference.name);
@ -284,16 +321,15 @@ class ForJitSerializer {
this.outputCtx, summary.type.reference,
this.serializeSummaryWithDeps(summary, metadata !));
}
});
}
exportAs.forEach((entry) => {
const symbol = entry.symbol;
if (ngModuleSymbols.has(symbol)) {
const jitExportAsName = summaryForJitName(entry.exportAs);
this.outputCtx.statements.push(
o.variable(jitExportAsName).set(this.serializeSummaryRef(symbol)).toDeclStmt(null, [
o.StmtModifier.Exported
]));
ngModuleSymbols.forEach((ngModuleSymbol) => {
if (this.summaryResolver.isLibraryFile(ngModuleSymbol.filePath)) {
let exportAs = exportAsBySymbol.get(ngModuleSymbol) || ngModuleSymbol.name;
const jitExportAsName = summaryForJitName(exportAs);
this.outputCtx.statements.push(o.variable(jitExportAsName)
.set(this.serializeSummaryRef(ngModuleSymbol))
.toDeclStmt(null, [o.StmtModifier.Exported]));
}
});
}
@ -378,22 +414,26 @@ class FromJsonDeserializer extends ValueTransformer {
deserialize(libraryFileName: string, json: string): {
moduleName: string | null,
summaries: Summary<StaticSymbol>[],
importAs: {symbol: StaticSymbol, importAs: string}[]
importAs: {symbol: StaticSymbol, importAs: StaticSymbol}[]
} {
const data: {moduleName: string | null, summaries: any[], symbols: any[]} = JSON.parse(json);
const importAs: {symbol: StaticSymbol, importAs: string}[] = [];
this.symbols = [];
data.symbols.forEach((serializedSymbol) => {
const symbol = this.symbolCache.get(
this.summaryResolver.fromSummaryFileName(serializedSymbol.filePath, libraryFileName),
serializedSymbol.name);
this.symbols.push(symbol);
if (serializedSymbol.importAs) {
importAs.push({symbol: symbol, importAs: serializedSymbol.importAs});
const allImportAs: {symbol: StaticSymbol, importAs: StaticSymbol}[] = [];
this.symbols = data.symbols.map(
(serializedSymbol) => this.symbolCache.get(
this.summaryResolver.fromSummaryFileName(serializedSymbol.filePath, libraryFileName),
serializedSymbol.name));
data.symbols.forEach((serializedSymbol, index) => {
const symbol = this.symbols[index];
const importAs = serializedSymbol.importAs;
if (typeof importAs === 'number') {
allImportAs.push({symbol, importAs: this.symbols[importAs]});
} else if (typeof importAs === 'string') {
allImportAs.push(
{symbol, importAs: this.symbolCache.get(ngfactoryFilePath(libraryFileName), importAs)});
}
});
const summaries = visitValue(data.summaries, this, null);
return {moduleName: data.moduleName, summaries, importAs};
const summaries = visitValue(data.summaries, this, null) as Summary<StaticSymbol>[];
return {moduleName: data.moduleName, summaries, importAs: allImportAs};
}
visitStringMap(map: {[key: string]: any}, context: any): any {
@ -407,3 +447,16 @@ class FromJsonDeserializer extends ValueTransformer {
}
}
}
function isCall(metadata: any): boolean {
return metadata && metadata.__symbolic === 'call';
}
function isFunctionCall(metadata: any): boolean {
return isCall(metadata) && metadata.expression instanceof StaticSymbol;
}
function isMethodCallOnVariable(metadata: any): boolean {
return isCall(metadata) && metadata.expression && metadata.expression.__symbolic === 'select' &&
metadata.expression.expression instanceof StaticSymbol;
}

View File

@ -59,3 +59,13 @@ export function summaryForJitName(symbolName: string): string {
export function stripSummaryForJitNameSuffix(symbolName: string): string {
return symbolName.replace(JIT_SUMMARY_NAME, '');
}
const LOWERED_SYMBOL = /\u0275\d+/;
export function isLoweredSymbol(name: string) {
return LOWERED_SYMBOL.test(name);
}
export function createLoweredSymbol(id: number): string {
return `\u0275${id}`;
}

View File

@ -39,6 +39,7 @@ export * from './aot/static_reflector';
export * from './aot/static_symbol';
export * from './aot/static_symbol_resolver';
export * from './aot/summary_resolver';
export {isLoweredSymbol, createLoweredSymbol} from './aot/util';
export {LazyRoute} from './aot/lazy_routes';
export * from './ast_path';
export * from './summary_resolver';

View File

@ -491,10 +491,11 @@ describe('compiler (unbundled Angular)', () => {
const libInput: MockDirectory = {
'lib': {
'base.ts': `
export class AValue {}
export type AType = {};
export class AClass {
constructor(a: AType) {}
constructor(a: AType, b: AValue) {}
}
`
}
@ -502,7 +503,7 @@ describe('compiler (unbundled Angular)', () => {
const appInput: MockDirectory = {
'app': {
'main.ts': `
export * from '../lib/base';
export {AClass} from '../lib/base';
`
}
};
@ -511,7 +512,105 @@ describe('compiler (unbundled Angular)', () => {
const {genFiles: appGenFiles} =
compile([appInput, libOutDir, angularSummaryFiles], {useSummaries: true});
const appNgFactory = appGenFiles.find((f) => f.genFileUrl === '/app/main.ngfactory.ts');
expect(toTypeScript(appNgFactory)).not.toContain('AType');
const appNgFactoryTs = toTypeScript(appNgFactory);
expect(appNgFactoryTs).not.toContain('AType');
expect(appNgFactoryTs).toContain('AValue');
});
it('should not reexport complex function calls', () => {
const libInput: MockDirectory = {
'lib': {
'base.ts': `
export class AClass {
constructor(arg: any) {}
static create(arg: any = null): AClass { return new AClass(arg); }
call(arg: any) {}
}
export function simple(arg: any) { return [arg]; }
export const ctor_arg = {};
export const ctor_call = new AClass(ctor_arg);
export const static_arg = {};
export const static_call = AClass.create(static_arg);
export const complex_arg = {};
export const complex_call = AClass.create().call(complex_arg);
export const simple_arg = {};
export const simple_call = simple(simple_arg);
`
}
};
const appInput: MockDirectory = {
'app': {
'main.ts': `
import {ctor_call, static_call, complex_call, simple_call} from '../lib/base';
export const calls = [ctor_call, static_call, complex_call, simple_call];
`,
}
};
const {outDir: libOutDir} = compile([libInput, angularSummaryFiles], {useSummaries: true});
const {genFiles: appGenFiles} =
compile([appInput, libOutDir, angularSummaryFiles], {useSummaries: true});
const appNgFactory = appGenFiles.find((f) => f.genFileUrl === '/app/main.ngfactory.ts');
const appNgFactoryTs = toTypeScript(appNgFactory);
// metadata of ctor calls is preserved, so we reexport the argument
expect(appNgFactoryTs).toContain('ctor_arg');
expect(appNgFactoryTs).toContain('ctor_call');
// metadata of static calls is preserved, so we reexport the argument
expect(appNgFactoryTs).toContain('static_arg');
expect(appNgFactoryTs).toContain('AClass');
expect(appNgFactoryTs).toContain('static_call');
// metadata of complex calls is elided, so we don't reexport the argument
expect(appNgFactoryTs).not.toContain('complex_arg');
expect(appNgFactoryTs).toContain('complex_call');
// metadata of simple calls is preserved, so we reexport the argument
expect(appNgFactoryTs).toContain('simple_arg');
expect(appNgFactoryTs).toContain('simple_call');
});
it('should not reexport already exported symbols except for lowered symbols', () => {
const libInput: MockDirectory = {
'lib': {
'base.ts': `
export const exportedVar = 1;
// A symbol introduced by lowering expressions
export const ɵ1 = 'lowered symbol';
`
}
};
const appInput: MockDirectory = {
'app': {
'main.ts': `export * from '../lib/base';`,
}
};
const {outDir: libOutDir} = compile([libInput, angularSummaryFiles], {useSummaries: true});
const {genFiles: appGenFiles} =
compile([appInput, libOutDir, angularSummaryFiles], {useSummaries: true});
const appNgFactory = appGenFiles.find((f) => f.genFileUrl === '/app/main.ngfactory.ts');
const appNgFactoryTs = toTypeScript(appNgFactory);
// we don't need to reexport exported symbols via the .ngfactory
// as we can refer to them via the reexport.
expect(appNgFactoryTs).not.toContain('exportedVar');
// although ɵ1 is reexported via `export *`, we still need to reexport it
// via the .ngfactory as tsickle expands `export *` into named exports,
// and doesn't know about our lowered symbols as we introduce them
// after the typecheck phase.
expect(appNgFactoryTs).toContain('ɵ1');
});
});

View File

@ -317,37 +317,96 @@ export function main() {
expect(summaries[1].metadata).toBe('someString');
});
it('should not create "importAs" names for reexported types in libraries', () => {
it('should not create "importAs" names for ctor arguments which are types of reexported classes in libraries',
() => {
init();
const externalSerialized = serializeSummaries(
'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver,
[
{
symbol: symbolCache.get('/tmp/external.ts', 'type'),
metadata: {__symbolic: 'interface'}
},
{
symbol: symbolCache.get('/tmp/external.ts', 'value'),
metadata: {__symbolic: 'class'}
},
{
symbol: symbolCache.get('/tmp/external.ts', 'reexportClass'),
metadata: {
__symbolic: 'class',
'members': {
'__ctor__': [{
'__symbolic': 'constructor',
'parameters': [
symbolCache.get('/tmp/external.ts', 'type'),
symbolCache.get('/tmp/external.ts', 'value'),
]
}]
}
}
},
],
[]);
expect(externalSerialized.exportAs).toEqual([]);
init({
'/tmp/external.ngsummary.json': externalSerialized.json,
});
const serialized = serializeSummaries(
'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, [{
symbol: symbolCache.get('/tmp/test.ts', 'mainClass'),
metadata: symbolCache.get('/tmp/external.d.ts', 'reexportClass'),
}],
[]);
const importAs =
deserializeSummaries(symbolCache, summaryResolver, 'someFile.d.ts', serialized.json)
.importAs;
expect(importAs).toEqual([
{
symbol: symbolCache.get('/tmp/external.d.ts', 'reexportClass'),
importAs: symbolCache.get('/tmp/test.d.ts', 'mainClass'),
},
{
symbol: symbolCache.get('/tmp/external.d.ts', 'value'),
importAs: symbolCache.get('someFile.ngfactory.d.ts', 'value_3'),
}
]);
});
it('should use existing reexports for "importAs" for symbols of libraries', () => {
init();
const externalSerialized = serializeSummaries(
'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver,
[
{symbol: symbolCache.get('/tmp/external.ts', 'value'), metadata: 'aValue'},
{
symbol: symbolCache.get('/tmp/external.ts', 'type'),
metadata: {__symbolic: 'interface'}
},
{
symbol: symbolCache.get('/tmp/external.ts', 'reexportType'),
metadata: symbolCache.get('/tmp/external.ts', 'type')
symbol: symbolCache.get('/tmp/external.ts', 'reexportValue'),
metadata: symbolCache.get('/tmp/external.ts', 'value')
},
],
[]);
expect(externalSerialized.exportAs).toEqual([]);
init({
'/tmp/external.ngsummary.json': externalSerialized.json,
});
const serialized = serializeSummaries(
'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, [{
symbol: symbolCache.get('/tmp/test.ts', 'mainType'),
metadata: symbolCache.get('/tmp/external.d.ts', 'reexportType'),
symbol: symbolCache.get('/tmp/test.ts', 'mainValue'),
metadata: symbolCache.get('/tmp/external.d.ts', 'reexportValue'),
}],
[]);
expect(serialized.exportAs).toEqual([]);
const importAs =
deserializeSummaries(symbolCache, summaryResolver, 'someFile.d.ts', serialized.json)
.importAs;
expect(importAs).toEqual([]);
expect(importAs).toEqual([{
symbol: symbolCache.get('/tmp/external.d.ts', 'value'),
importAs: symbolCache.get('/tmp/test.d.ts', 'mainValue'),
}]);
});
it('should create "importAs" names for non source symbols', () => {
it('should create reexports in the ngfactory for symbols of libraries', () => {
init();
const serialized = serializeSummaries(
'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, [{
@ -366,9 +425,10 @@ export function main() {
const deserialized =
deserializeSummaries(symbolCache, summaryResolver, 'someFile.d.ts', serialized.json);
// Note: no entry for the symbol with members!
expect(deserialized.importAs).toEqual([
{symbol: symbolCache.get('/tmp/external.d.ts', 'lib'), importAs: 'lib_1'}
]);
expect(deserialized.importAs).toEqual([{
symbol: symbolCache.get('/tmp/external.d.ts', 'lib'),
importAs: symbolCache.get('someFile.ngfactory.d.ts', 'lib_1')
}]);
});
});
}