honeymoose
/
angular-cn
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

refactor(compiler): allow disabling external symbol factory reexports (#28594) 

Currently external static symbols which are referenced by AOT
compiler generated code, will be re-exported in the corresponding
`.ngfactory` files.

This way of handling the symbol resolution has been introduced in
favor of avoding dynamically generated module dependencies. This
behavior therefore avoids any strict dependency failures.

Read more about a particular scenario here: https://github.com/angular/angular/issues/25644#issuecomment-458354439

Now with `ngtsc`, this behavior has changed since `ngtsc` just
introduces these module dependencies in order to properly reference
the external symbol from its original location (also eliminating the need
for factories). Similarly we should provide a way to use the same
behavior with `ngc` because the downside of using the re-exported symbol
resolution is that user-code transformations (e.g. the `ngInjectableDef`
metadata which is added to the user source code), can resolve external
symbols to previous factory symbol re-exports. This is a critical issue
because it means that the actual JIT code references factory files in order
to access external symbols. This means that the generated output cannot
shipped to NPM without shipping the referenced factory files.

A specific example has been reported here: https://github.com/angular/angular/issues/25644#issue-353554070

PR Close #28594
This commit is contained in:
Paul Gschwendtner 2019-02-09 19:13:18 +01:00 committed by Miško Hevery
parent 9ec8aa5eb7
commit 872a3656fe
4 changed files with 78 additions and 5 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
packages/compiler/src/aot/compiler.ts
View File

@ -553,7 +553,7 @@ export class AotCompiler {
null;
const {json, exportAs} = serializeSummaries(
srcFileName, forJitOutputCtx, this._summaryResolver, this._symbolResolver, symbolSummaries,
typeData);
typeData, this._options.createExternalSymbolFactoryReexports);
exportAs.forEach((entry) => {
ngFactoryCtx.statements.push(
o.variable(entry.exportAs).set(ngFactoryCtx.importExpr(entry.symbol)).toDeclStmt(null, [

1
packages/compiler/src/aot/compiler_options.ts
View File

@ -20,4 +20,5 @@ export interface AotCompilerOptions {
allowEmptyCodegenFiles?: boolean;
strictInjectionParameters?: boolean;
enableIvy?: boolean|'ngtsc'|'tsc';
createExternalSymbolFactoryReexports?: boolean;
}

27
packages/compiler/src/aot/summary_serializer.ts
View File

@ -21,7 +21,9 @@ export function serializeSummaries(
summary: CompileTypeSummary,
metadata: CompileNgModuleMetadata | CompileDirectiveMetadata | CompilePipeMetadata |
CompileTypeMetadata
}[]): {json: string, exportAs: {symbol: StaticSymbol, exportAs: string}[]} {
}[],
createExternalSymbolReexports =
true): {json: string, exportAs: {symbol: StaticSymbol, exportAs: string}[]} {
const toJsonSerializer = new ToJsonSerializer(symbolResolver, summaryResolver, srcFileName);
// for symbols, we use everything except for the class metadata itself
@ -36,7 +38,7 @@ export function serializeSummaries(
toJsonSerializer.addSummary(
{symbol: summary.type.reference, metadata: undefined, type: summary});
});
const {json, exportAs} = toJsonSerializer.serialize();
const {json, exportAs} = toJsonSerializer.serialize(createExternalSymbolReexports);
if (forJitCtx) {
const forJitSerializer = new ForJitSerializer(forJitCtx, symbolResolver, summaryResolver);
types.forEach(({summary, metadata}) => { forJitSerializer.addSourceType(summary, metadata); });
@ -178,7 +180,14 @@ class ToJsonSerializer extends ValueTransformer {
}
}
serialize(): {json: string, exportAs: {symbol: StaticSymbol, exportAs: string}[]} {
/**
* @param createExternalSymbolReexports Whether external static symbols should be re-exported.
* This can be enabled if external symbols should be re-exported by the current module in
* order to avoid dynamically generated module dependencies which can break strict dependency
* enforcements (as in Google3). Read more here: https://github.com/angular/angular/issues/25644
*/
serialize(createExternalSymbolReexports: boolean):
{json: string, exportAs: {symbol: StaticSymbol, exportAs: string}[]} {
const exportAs: {symbol: StaticSymbol, exportAs: string}[] = [];
const json = JSON.stringify({
moduleName: this.moduleName,
@ -189,8 +198,18 @@ class ToJsonSerializer extends ValueTransformer {
if (this.summaryResolver.isLibraryFile(symbol.filePath)) {
const reexportSymbol = this.reexportedBy.get(symbol);
if (reexportSymbol) {
// In case the given external static symbol is already manually exported by the
// user, we just proxy the external static symbol reference to the manual export.
// This ensures that the AOT compiler imports the external symbol through the
// user export and does not introduce another dependency which is not needed.
importAs = this.indexBySymbol.get(reexportSymbol) !;
} else {
} else if (createExternalSymbolReexports) {
// In this case, the given external static symbol is *not* manually exported by
// the user, and we manually create a re-export in the factory file so that we
// don't introduce another module dependency. This is useful when running within
// Bazel so that the AOT compiler does not introduce any module dependencies
// which can break the strict dependency enforcement. (e.g. as in Google3)
// Read more about this here: https://github.com/angular/angular/issues/25644
const summary = this.unprocessedSymbolSummariesBySymbol.get(symbol);
if (!summary || !summary.metadata || summary.metadata.__symbolic !== 'interface') {
importAs = `${symbol.name}_${index}`;

53
packages/compiler/test/aot/summary_serializer_spec.ts
View File

@ -466,5 +466,58 @@ import {MockAotSummaryResolverHost, createMockOutputContext} from './summary_res
expect(serialized.json).not.toContain('error');
});
});
describe('createExternalSymbolReexports disabled', () => {
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', 'reexportValue'),
metadata: symbolCache.get('/tmp/external.ts', 'value')
},
],
[], false);
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', '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([{
symbol: symbolCache.get('/tmp/external.d.ts', 'value'),
importAs: symbolCache.get('/tmp/test.d.ts', 'mainValue'),
}]);
});
it('should not create reexports in the ngfactory for external symbols', () => {
init();
const serialized = serializeSummaries(
'someFile.ts', createMockOutputContext(), summaryResolver, symbolResolver, [{
symbol: symbolCache.get('/tmp/test.ts', 'main'),
metadata: [
symbolCache.get('/tmp/external.d.ts', 'lib'),
symbolCache.get('/tmp/external.d.ts', 'lib', ['someMember']),
]
}],
[], false);
expect(serialized.exportAs.length)
.toBe(0, 'Expected no external symbols to be re-exported.');
const deserialized =
deserializeSummaries(symbolCache, summaryResolver, 'someFile.d.ts', serialized.json);
expect(deserialized.importAs.length)
.toBe(0, 'Expected no symbols that can be imported from a re-exported location');
});
});
});
}