fix(ngcc): render UMD global imports correctly (#34012)

The current UMD rendering formatter did not handle
a number of corner cases, such as imports from namespaced
packages.

PR Close #34012
This commit is contained in:
Pete Bacon Darwin 2019-11-23 17:09:59 +00:00 committed by Matias Niemelä
parent 51a56bc4c6
commit 44225e4010
2 changed files with 48 additions and 2 deletions

View File

@ -240,8 +240,34 @@ function isCommaExpression(value: ts.Node): value is ts.BinaryExpression {
return ts.isBinaryExpression(value) && value.operatorToken.kind === ts.SyntaxKind.CommaToken; return ts.isBinaryExpression(value) && value.operatorToken.kind === ts.SyntaxKind.CommaToken;
} }
function getGlobalIdentifier(i: Import) { /**
return i.specifier.replace('@angular/', 'ng.').replace(/^\//, ''); * Compute a global identifier for the given import (`i`).
*
* The identifier used to access a package when using the "global" form of a UMD bundle usually
* follows a special format where snake-case is conveted to camelCase and path separators are
* converted to dots. In addition there are special cases such as `@angular` is mapped to `ng`.
*
* For example
*
* * `@ns/package/entry-point` => `ns.package.entryPoint`
* * `@angular/common/testing` => `ng.common.testing`
* * `@angular/platform-browser-dynamic` => `ng.platformBrowserDynamic`
*
* It is possible for packages to specify completely different identifiers for attaching the package
* to the global, and so there is no guaranteed way to compute this.
* Currently, this approach appears to work for the known scenarios; also it is not known how common
* it is to use globals for importing packages.
*
* If it turns out that there are packages that are being used via globals, where this approach
* fails, we should consider implementing a configuration based solution, similar to what would go
* in a rollup configuration for mapping import paths to global indentifiers.
*/
function getGlobalIdentifier(i: Import): string {
return i.specifier.replace(/^@angular\//, 'ng.')
.replace(/^@/, '')
.replace(/\//g, '.')
.replace(/[-_]+(.?)/g, (_, c) => c.toUpperCase())
.replace(/^./, c => c.toLowerCase());
} }
function find<T>(node: ts.Node, test: (node: ts.Node) => node is ts.Node & T): T|undefined { function find<T>(node: ts.Node, test: (node: ts.Node) => node is ts.Node & T): T|undefined {

View File

@ -240,6 +240,25 @@ typeof define === 'function' && define.amd ? define('file', ['exports','/tslib',
`(factory(global.file,global.someSideEffect,global.localDep,global.ng.core,global.ng.core,global.ng.common));`); `(factory(global.file,global.someSideEffect,global.localDep,global.ng.core,global.ng.core,global.ng.common));`);
}); });
it('should remap import identifiers to valid global properties', () => {
const {renderer, program} = setup(PROGRAM);
const file = getSourceFileOrError(program, _('/node_modules/test-package/some/file.js'));
const output = new MagicString(PROGRAM.contents);
renderer.addImports(
output,
[
{specifier: '@ngrx/store', qualifier: 'i0'},
{specifier: '@angular/platform-browser-dynamic', qualifier: 'i1'},
{specifier: '@angular/common/testing', qualifier: 'i2'},
{specifier: '@angular-foo/package', qualifier: 'i3'}
],
file);
expect(output.toString())
.toContain(
`(factory(global.file,global.someSideEffect,global.localDep,global.ng.core,` +
`global.ngrx.store,global.ng.platformBrowserDynamic,global.ng.common.testing,global.angularFoo.package));`);
});
it('should append the given imports into the global initialization, if it has a global/self initializer', it('should append the given imports into the global initialization, if it has a global/self initializer',
() => { () => {
const {renderer, program} = setup(PROGRAM_WITH_GLOBAL_INITIALIZER); const {renderer, program} = setup(PROGRAM_WITH_GLOBAL_INITIALIZER);
@ -256,6 +275,7 @@ typeof define === 'function' && define.amd ? define('file', ['exports','/tslib',
.toContain( .toContain(
`(global = global || self, factory(global.file,global.someSideEffect,global.localDep,global.ng.core,global.ng.core,global.ng.common));`); `(global = global || self, factory(global.file,global.someSideEffect,global.localDep,global.ng.core,global.ng.core,global.ng.common));`);
}); });
it('should append the given imports as parameters into the factory function definition', it('should append the given imports as parameters into the factory function definition',
() => { () => {
const {renderer, program} = setup(PROGRAM); const {renderer, program} = setup(PROGRAM);