Navigation Menu

Skip to content

Commit

Permalink
fix(compiler-cli): incorrect metadata bundle for multiple unnamed re-…
Browse files Browse the repository at this point in the history
…exports (angular#29360)

Currently if an Angular library has multiple unnamed module re-exports, NGC will
generate incorrect metdata if the project is using the flat-module bundle option.

e.g.

_public-api.ts_
```ts
export * from '@mypkg/secondary1';
export * from '@mypkg/secondary2';
```

There are clearly two unnamed re-exports in the `public-api.ts` file. NGC right now
accidentally overwrites all previous re-exports with the last one. Resulting in the
generated metadata only containing a reference to `@mypkg/secondary2`.

This is problematic as it is common for primary library entry-points to have
multiple re-exports (e.g. Material re-exporting all public symbols; or flex-layout
exporting all public symbols from their secondary entry-points).

Currently Angular Material works around this issue by manually creating
a metadata file that declares the re-exports from all unnamed re-exports.

(see: https://github.com/angular/material2/blob/master/tools/package-tools/build-release.ts#L78-L85)

This workaround works fine currently, but is no longer easily integrated when
building the package output with Bazel. In order to be able to build such
libraries with Bazel (Material/flex-layout), we need to make sure that NGC
generates the proper flat-module metadata bundle.

PR Close angular#29360
  • Loading branch information
devversion authored and wKoza committed Apr 17, 2019
1 parent ca678ff commit b42ca21
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 1 deletion.
8 changes: 7 additions & 1 deletion packages/compiler-cli/src/metadata/bundler.ts
Expand Up @@ -180,6 +180,7 @@ export class MetadataBundler {

// Export all the re-exports from this module
if (module && module.exports) {
let unnamedModuleExportsIdx = 0;
for (const exportDeclaration of module.exports) {
const exportFrom = resolveModule(exportDeclaration.from, moduleName);
// Record all the exports from the module even if we don't use it directly.
Expand All @@ -202,7 +203,12 @@ export class MetadataBundler {
// Re-export all the symbols from the module
const exportedSymbols = this.exportAll(exportFrom);
for (const exportedSymbol of exportedSymbols) {
const name = exportedSymbol.name;
// In case the exported symbol does not have a name, we need to give it an unique
// name for the current module. This is necessary because there can be multiple
// unnamed re-exports in a given module.
const name = exportedSymbol.name === '*' ?
`unnamed_reexport_${unnamedModuleExportsIdx++}` :
exportedSymbol.name;
exportSymbol(exportedSymbol, name);
}
}
Expand Down
15 changes: 15 additions & 0 deletions packages/compiler-cli/test/metadata/bundler_spec.ts
Expand Up @@ -422,6 +422,21 @@ describe('metadata bundler', () => {
expect(result.metadata.origins !['E']).toBeUndefined();
});

it('should be able to bundle a library with multiple unnamed re-exports', () => {
const host = new MockStringBundlerHost('/', {
'public-api.ts': `
export * from '@mypkg/secondary1';
export * from '@mypkg/secondary2';
`,
});

const bundler = new MetadataBundler('/public-api', undefined, host);
const result = bundler.getMetadataBundle();
expect(result.metadata.exports).toEqual([
{from: '@mypkg/secondary1'}, {from: '@mypkg/secondary2'}
]);
});

it('should be able to de-duplicate symbols of re-exported modules', () => {
const host = new MockStringBundlerHost('/', {
'public-api.ts': `
Expand Down

0 comments on commit b42ca21

Please sign in to comment.