From e31a76ce24c8c3883df791d4409e136196973165 Mon Sep 17 00:00:00 2001 From: Tobias Bosch Date: Sun, 24 Sep 2017 11:19:01 -0700 Subject: [PATCH] fix(compiler): allow to use lowering with `export *`. Previously, we generated incorrect code when a file had lowerings and also exported another file via `export *` that also had lowerings in it. --- packages/compiler-cli/test/ngc_spec.ts | 32 ++++++++++ .../src/aot/static_symbol_resolver.ts | 60 ++++++++++--------- .../test/aot/static_symbol_resolver_spec.ts | 24 ++++++-- 3 files changed, 83 insertions(+), 33 deletions(-) diff --git a/packages/compiler-cli/test/ngc_spec.ts b/packages/compiler-cli/test/ngc_spec.ts index 9071939714..73513d4d3f 100644 --- a/packages/compiler-cli/test/ngc_spec.ts +++ b/packages/compiler-cli/test/ngc_spec.ts @@ -711,6 +711,38 @@ describe('ngc transformer command-line', () => { expect(main(['-p', basePath], errorSpy)).toBe(0); shouldExist('module.js'); }); + + it('should allow to use lowering with export *', () => { + write('mymodule.ts', ` + import {NgModule} from '@angular/core'; + + export * from './util'; + + // Note: the lamda will be lowered into an exported expression + @NgModule({providers: [{provide: 'aToken', useValue: () => 2}]}) + export class MyModule {} + `); + write('util.ts', ` + // Note: The lamda will be lowered into an exported expression + const x = () => 2; + + export const y = x; + `); + + expect(compile()).toEqual(0); + + const mymoduleSource = fs.readFileSync(path.resolve(outDir, 'mymodule.js'), 'utf8'); + expect(mymoduleSource).toContain('ɵ0'); + + const utilSource = fs.readFileSync(path.resolve(outDir, 'util.js'), 'utf8'); + expect(utilSource).toContain('ɵ0'); + + const mymoduleNgFactoryJs = + fs.readFileSync(path.resolve(outDir, 'mymodule.ngfactory.js'), 'utf8'); + // check that the generated code refers to ɵ0 from mymodule, and not from util! + expect(mymoduleNgFactoryJs).toContain(`import * as i1 from "./mymodule"`); + expect(mymoduleNgFactoryJs).toContain(`"aToken", i1.ɵ0`); + }); }); function writeFlatModule(outFile: string) { diff --git a/packages/compiler/src/aot/static_symbol_resolver.ts b/packages/compiler/src/aot/static_symbol_resolver.ts index 884213b97d..1d890590d2 100644 --- a/packages/compiler/src/aot/static_symbol_resolver.ts +++ b/packages/compiler/src/aot/static_symbol_resolver.ts @@ -292,34 +292,6 @@ export class StaticSymbolResolver { this.resolvedFilePaths.add(filePath); const resolvedSymbols: ResolvedStaticSymbol[] = []; const metadata = this.getModuleMetadata(filePath); - if (metadata['metadata']) { - // handle direct declarations of the symbol - const topLevelSymbolNames = - new Set(Object.keys(metadata['metadata']).map(unescapeIdentifier)); - const origins: {[index: string]: string} = metadata['origins'] || {}; - Object.keys(metadata['metadata']).forEach((metadataKey) => { - const symbolMeta = metadata['metadata'][metadataKey]; - const name = unescapeIdentifier(metadataKey); - - const symbol = this.getStaticSymbol(filePath, name); - - const origin = origins.hasOwnProperty(metadataKey) && origins[metadataKey]; - if (origin) { - // If the symbol is from a bundled index, use the declaration location of the - // symbol so relative references (such as './my.html') will be calculated - // correctly. - const originFilePath = this.resolveModule(origin, filePath); - if (!originFilePath) { - this.reportError( - new Error(`Couldn't resolve original symbol for ${origin} from ${filePath}`)); - } else { - this.symbolResourcePaths.set(symbol, originFilePath); - } - } - resolvedSymbols.push( - this.createResolvedSymbol(symbol, filePath, topLevelSymbolNames, symbolMeta)); - }); - } // handle the symbols in one of the re-export location if (metadata['exports']) { @@ -358,6 +330,38 @@ export class StaticSymbolResolver { } } } + + // handle the actual metadata. Has to be after the exports + // as there migth be collisions in the names, and we want the symbols + // of the current module to win ofter reexports. + if (metadata['metadata']) { + // handle direct declarations of the symbol + const topLevelSymbolNames = + new Set(Object.keys(metadata['metadata']).map(unescapeIdentifier)); + const origins: {[index: string]: string} = metadata['origins'] || {}; + Object.keys(metadata['metadata']).forEach((metadataKey) => { + const symbolMeta = metadata['metadata'][metadataKey]; + const name = unescapeIdentifier(metadataKey); + + const symbol = this.getStaticSymbol(filePath, name); + + const origin = origins.hasOwnProperty(metadataKey) && origins[metadataKey]; + if (origin) { + // If the symbol is from a bundled index, use the declaration location of the + // symbol so relative references (such as './my.html') will be calculated + // correctly. + const originFilePath = this.resolveModule(origin, filePath); + if (!originFilePath) { + this.reportError( + new Error(`Couldn't resolve original symbol for ${origin} from ${filePath}`)); + } else { + this.symbolResourcePaths.set(symbol, originFilePath); + } + } + resolvedSymbols.push( + this.createResolvedSymbol(symbol, filePath, topLevelSymbolNames, symbolMeta)); + }); + } resolvedSymbols.forEach( (resolvedSymbol) => this.resolvedSymbols.set(resolvedSymbol.symbol, resolvedSymbol)); this.symbolFromFile.set(filePath, resolvedSymbols.map(resolvedSymbol => resolvedSymbol.symbol)); diff --git a/packages/compiler/test/aot/static_symbol_resolver_spec.ts b/packages/compiler/test/aot/static_symbol_resolver_spec.ts index f2119e64d8..eb6032f3f3 100644 --- a/packages/compiler/test/aot/static_symbol_resolver_spec.ts +++ b/packages/compiler/test/aot/static_symbol_resolver_spec.ts @@ -124,6 +124,7 @@ describe('StaticSymbolResolver', () => { symbolResolver.getStaticSymbol('/tmp/src/reexport/src/origin1.d.ts', 'One'), symbolResolver.getStaticSymbol('/tmp/src/reexport/src/origin1.d.ts', 'Two'), symbolResolver.getStaticSymbol('/tmp/src/reexport/src/origin1.d.ts', 'Three'), + symbolResolver.getStaticSymbol('/tmp/src/reexport/src/origin1.d.ts', 'Six'), ]); }); @@ -132,8 +133,9 @@ describe('StaticSymbolResolver', () => { symbolResolver.getStaticSymbol('/tmp/src/reexport/reexport.d.ts', 'One'), symbolResolver.getStaticSymbol('/tmp/src/reexport/reexport.d.ts', 'Two'), symbolResolver.getStaticSymbol('/tmp/src/reexport/reexport.d.ts', 'Four'), + symbolResolver.getStaticSymbol('/tmp/src/reexport/reexport.d.ts', 'Six'), symbolResolver.getStaticSymbol('/tmp/src/reexport/reexport.d.ts', 'Five'), - symbolResolver.getStaticSymbol('/tmp/src/reexport/reexport.d.ts', 'Thirty') + symbolResolver.getStaticSymbol('/tmp/src/reexport/reexport.d.ts', 'Thirty'), ]); }); @@ -287,8 +289,9 @@ describe('StaticSymbolResolver', () => { .toBe(symbolCache.get('/test2.d.ts', '__a__')); expect(symbolResolver.getSymbolsOf('/test.ts')).toEqual([ - symbolCache.get('/test.ts', '__x__'), symbolCache.get('/test.ts', '__y__'), - symbolCache.get('/test.ts', '__b__') + symbolCache.get('/test.ts', '__b__'), + symbolCache.get('/test.ts', '__x__'), + symbolCache.get('/test.ts', '__y__'), ]); }); @@ -356,6 +359,14 @@ describe('StaticSymbolResolver', () => { expect(symbol2.filePath).toEqual('/tmp/src/reexport/src/origin30.d.ts'); }); + it('should prefer names in the file over reexports', () => { + const metadata = symbolResolver + .resolveSymbol(symbolResolver.getSymbolByModule( + './reexport/reexport', 'Six', '/tmp/src/main.ts')) + .metadata; + expect(metadata.__symbolic).toBe('class'); + }); + it('should cache tracing a named export', () => { const moduleNameToFileNameSpy = spyOn(host, 'moduleNameToFileName').and.callThrough(); const getMetadataForSpy = spyOn(host, 'getMetadataFor').and.callThrough(); @@ -494,9 +505,11 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = { '/tmp/src/reexport/reexport.d.ts': { __symbolic: 'module', version: 3, - metadata: {}, + metadata: { + Six: {__symbolic: 'class'}, + }, exports: [ - {from: './src/origin1', export: ['One', 'Two', {name: 'Three', as: 'Four'}]}, + {from: './src/origin1', export: ['One', 'Two', {name: 'Three', as: 'Four'}, 'Six']}, {from: './src/origin5'}, {from: './src/reexport2'} ] }, @@ -507,6 +520,7 @@ const DEFAULT_TEST_DATA: {[key: string]: any} = { One: {__symbolic: 'class'}, Two: {__symbolic: 'class'}, Three: {__symbolic: 'class'}, + Six: {__symbolic: 'class'}, }, }, '/tmp/src/reexport/src/origin5.d.ts': {