Skip to content

Commit

Permalink
fix(ngcc): support UMD global factory in comma lists (#32709)
Browse files Browse the repository at this point in the history
Previously we were looking for a global factory call that looks like:

```ts
(factory((global.ng = global.ng || {}, global.ng.common = {}), global.ng.core))"
```

but in some cases it looks like:

```ts
(global = global || self, factory((global.ng = global.ng || {}, global.ng.common = {}), global.ng.core))"
```

Note the `global = global || self` at the start of the statement.

This commit makes the test when finding the global factory
function call resilient to being in a comma list.

PR Close #32709
  • Loading branch information
petebacondarwin authored and AndrewKushnir committed Sep 17, 2019
1 parent 894c4b5 commit e5a3de5
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 3 deletions.
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -204,15 +204,21 @@ function isAmdConditional(value: ts.Node): value is AmdConditional {
*/ */
function isGlobalFactoryCall(value: ts.Node): value is ts.CallExpression { function isGlobalFactoryCall(value: ts.Node): value is ts.CallExpression {
if (ts.isCallExpression(value) && !!value.parent) { if (ts.isCallExpression(value) && !!value.parent) {
// Be resilient to the value being part of a comma list
value = isCommaExpression(value.parent) ? value.parent : value;
// Be resilient to the value being inside parentheses // Be resilient to the value being inside parentheses
const expression = ts.isParenthesizedExpression(value.parent) ? value.parent : value; value = ts.isParenthesizedExpression(value.parent) ? value.parent : value;
return !!expression.parent && ts.isConditionalExpression(expression.parent) && return !!value.parent && ts.isConditionalExpression(value.parent) &&
expression.parent.whenFalse === expression; value.parent.whenFalse === value;
} else { } else {
return false; return false;
} }
} }


function isCommaExpression(value: ts.Node): value is ts.BinaryExpression {
return ts.isBinaryExpression(value) && value.operatorToken.kind === ts.SyntaxKind.CommaToken;
}

function getGlobalIdentifier(i: Import) { function getGlobalIdentifier(i: Import) {
return i.specifier.replace('@angular/', 'ng.').replace(/^\//, ''); return i.specifier.replace('@angular/', 'ng.').replace(/^\//, '');
} }
Expand Down
Original file line number Original file line Diff line number Diff line change
Expand Up @@ -50,10 +50,21 @@ runInEachFileSystem(() => {
let _: typeof absoluteFrom; let _: typeof absoluteFrom;
let PROGRAM: TestFile; let PROGRAM: TestFile;
let PROGRAM_DECORATE_HELPER: TestFile; let PROGRAM_DECORATE_HELPER: TestFile;
let PROGRAM_WITH_GLOBAL_INITIALIZER: TestFile;


beforeEach(() => { beforeEach(() => {
_ = absoluteFrom; _ = absoluteFrom;


PROGRAM_WITH_GLOBAL_INITIALIZER = {
name: _('/node_modules/test-package/some/file.js'),
contents: `
(function (global, factory) {
typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports,require('some-side-effect'),require('/local-dep'),require('@angular/core')) :
typeof define === 'function' && define.amd ? define('file', ['exports','some-side-effect','/local-dep','@angular/core'], factory) :
(global = global || self, factory(global.file,global.someSideEffect,global.localDep,global.ng.core));
}(this, (function (exports,someSideEffect,localDep,core) {'use strict'; })));`
};

PROGRAM = { PROGRAM = {
name: _('/node_modules/test-package/some/file.js'), name: _('/node_modules/test-package/some/file.js'),
contents: ` contents: `
Expand Down Expand Up @@ -228,6 +239,22 @@ 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 append the given imports into the global initialization, if it has a global/self initializer',
() => {
const {renderer, program} = setup(PROGRAM_WITH_GLOBAL_INITIALIZER);
const file = getSourceFileOrError(program, _('/node_modules/test-package/some/file.js'));
const output = new MagicString(file.text);
renderer.addImports(
output,
[
{specifier: '@angular/core', qualifier: 'i0'},
{specifier: '@angular/common', qualifier: 'i1'}
],
file);
expect(output.toString())
.toContain(
`(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);
Expand Down

0 comments on commit e5a3de5

Please sign in to comment.