Sitelet https://github.com/angular/angular/commit/13c4a7b
Skip to content

Commit 13c4a7b

Browse files
petebacondarwinatscott
petebacondarwin
authored and
atscott
committed
fix(ngcc): map exports to the current module in UMD files (#38959) (#39272)
UMD files export values by assigning them to an `exports` variable. When evaluating expressions ngcc was failing to cope with expressions like `exports.MyComponent`. This commit fixes the `UmdReflectionHost.getDeclarationOfIdentifier()` method to map the `exports` variable to the current source file. PR Close #38959 PR Close #39272
1 parent 70e85d2 commit 13c4a7b

2 files changed

Lines changed: 171 additions & 2 deletions

File tree

‎packages/compiler-cli/ngcc/src/host/umd_host.ts‎

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
4444
}
4545

4646
getDeclarationOfIdentifier(id: ts.Identifier): Declaration|null {
47-
return this.getUmdModuleDeclaration(id) || this.getUmdDeclaration(id) ||
48-
super.getDeclarationOfIdentifier(id);
47+
return this.getExportsDeclaration(id) || this.getUmdModuleDeclaration(id) ||
48+
this.getUmdDeclaration(id) || super.getDeclarationOfIdentifier(id);
4949
}
5050

5151
getExportsOfModule(module: ts.Node): Map<string, Declaration>|null {
@@ -249,6 +249,29 @@ export class UmdReflectionHost extends Esm5ReflectionHost {
249249
return {...declaration, viaModule, known: getTsHelperFnFromIdentifier(id)};
250250
}
251251

252+
private getExportsDeclaration(id: ts.Identifier): Declaration|null {
253+
if (!isExportIdentifier(id)) {
254+
return null;
255+
}
256+
257+
// Sadly, in the case of `exports.foo = bar`, we can't use `this.findUmdImportParameter(id)` to
258+
// check whether this `exports` is from the IIFE body arguments, because
259+
// `this.checker.getSymbolAtLocation(id)` will return the symbol for the `foo` identifier rather
260+
// than the `exports` identifier.
261+
//
262+
// Instead we search the symbols in the current local scope.
263+
const exportsSymbol = this.checker.getSymbolsInScope(id, ts.SymbolFlags.Variable)
264+
.find(symbol => symbol.name === 'exports');
265+
if (exportsSymbol !== undefined &&
266+
!ts.isFunctionExpression(exportsSymbol.valueDeclaration.parent)) {
267+
// There is an `exports` symbol in the local scope that is not a function parameter.
268+
// So this `exports` identifier must be a local variable and does not represent the module.
269+
return {node: exportsSymbol.valueDeclaration, viaModule: null, known: null, identity: null};
270+
}
271+
272+
return {node: id.getSourceFile(), viaModule: null, known: null, identity: null};
273+
}
274+
252275
private getUmdModuleDeclaration(id: ts.Identifier): Declaration|null {
253276
const importPath = this.getImportPathFromParameter(id) || this.getImportPathFromRequireCall(id);
254277
if (importPath === null) {
@@ -370,3 +393,10 @@ function getRequiredModulePath(wrapperFn: ts.FunctionExpression, paramIndex: num
370393
}
371394
}
372395
}
396+
397+
/**
398+
* Is the `node` an identifier with the name "exports"?
399+
*/
400+
export function isExportIdentifier(node: ts.Node): node is ts.Identifier {
401+
return ts.isIdentifier(node) && node.text === 'exports';
402+
}

‎packages/compiler-cli/ngcc/test/host/umd_host_spec.ts‎

Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {MockLogger} from '../../../src/ngtsc/logging/testing';
1414
import {ClassMemberKind, ConcreteDeclaration, CtorParameter, DownleveledEnum, Import, InlineDeclaration, isNamedClassDeclaration, isNamedFunctionDeclaration, isNamedVariableDeclaration, KnownDeclaration, TypeScriptReflectionHost, TypeValueReferenceKind} from '../../../src/ngtsc/reflection';
1515
import {getDeclaration} from '../../../src/ngtsc/testing';
1616
import {loadFakeCore, loadTestFiles} from '../../../test/helpers';
17+
import {isExportsStatement} from '../../src/host/commonjs_umd_utils';
1718
import {DelegatingReflectionHost} from '../../src/host/delegating_host';
1819
import {getIifeBody} from '../../src/host/esm2015_host';
1920
import {NgccReflectionHost} from '../../src/host/ngcc_host';
@@ -34,6 +35,7 @@ runInEachFileSystem(() => {
3435
let SIMPLE_CLASS_FILE: TestFile;
3536
let FOO_FUNCTION_FILE: TestFile;
3637
let INLINE_EXPORT_FILE: TestFile;
38+
let EXPORTS_IDENTIFIERS_FILE: TestFile;
3739
let INVALID_DECORATORS_FILE: TestFile;
3840
let INVALID_DECORATOR_ARGS_FILE: TestFile;
3941
let INVALID_PROP_DECORATORS_FILE: TestFile;
@@ -238,6 +240,33 @@ runInEachFileSystem(() => {
238240
`,
239241
};
240242

243+
EXPORTS_IDENTIFIERS_FILE = {
244+
name: _('/exports_identifiers.js'),
245+
contents: `
246+
(function (global, factory) {
247+
typeof exports === 'object' && typeof module !== 'undefined' ? factory(exports, require('@angular/core')) :
248+
typeof define === 'function' && define.amd ? define('foo_function', ['exports', '@angular/core'], factory) :
249+
(factory(global.inline_export,global.ng.core));
250+
}(this, (function (exports,core) { 'use strict';
251+
var x = exports;
252+
exports.foo = 42;
253+
254+
function simpleFn() {
255+
exports.foo = 42;
256+
}
257+
258+
function localVar() {
259+
var exports = {};
260+
exports.foo = 42;
261+
}
262+
263+
function exportsArg(exports) {
264+
exports.foo = 42;
265+
}
266+
})));
267+
`,
268+
};
269+
241270
INVALID_DECORATORS_FILE = {
242271
name: _('/invalid_decorators.js'),
243272
contents: `
@@ -2078,6 +2107,111 @@ runInEachFileSystem(() => {
20782107
expect(actualDeclaration!.viaModule).toBe('@angular/core');
20792108
});
20802109

2110+
it('should return the source file as the declaration of a standalone "exports" identifier',
2111+
() => {
2112+
loadTestFiles([EXPORTS_IDENTIFIERS_FILE]);
2113+
const bundle = makeTestBundleProgram(EXPORTS_IDENTIFIERS_FILE.name);
2114+
const host = createHost(bundle, new UmdReflectionHost(new MockLogger(), false, bundle));
2115+
const xVar = getDeclaration(
2116+
bundle.program, EXPORTS_IDENTIFIERS_FILE.name, 'x', ts.isVariableDeclaration);
2117+
const exportsIdentifier = xVar.initializer;
2118+
if (exportsIdentifier === undefined || !ts.isIdentifier(exportsIdentifier)) {
2119+
throw new Error('Bad test - unable to find `var x = exports;` statement');
2120+
}
2121+
const exportsDeclaration = host.getDeclarationOfIdentifier(exportsIdentifier);
2122+
expect(exportsDeclaration).not.toBeNull();
2123+
expect(exportsDeclaration!.node).toBe(bundle.file);
2124+
});
2125+
2126+
it('should return the source file as the declaration of "exports" in the LHS of a property access',
2127+
() => {
2128+
loadTestFiles([EXPORTS_IDENTIFIERS_FILE]);
2129+
const bundle = makeTestBundleProgram(EXPORTS_IDENTIFIERS_FILE.name);
2130+
const host = createHost(bundle, new UmdReflectionHost(new MockLogger(), false, bundle));
2131+
const exportsStatement = walkForNode(bundle.file, isExportsStatement);
2132+
if (exportsStatement === undefined) {
2133+
throw new Error('Bad test - unable to find `exports.foo = 42;` statement');
2134+
}
2135+
const exportsIdentifier = exportsStatement.expression.left.expression;
2136+
const exportDeclaration = host.getDeclarationOfIdentifier(exportsIdentifier);
2137+
expect(exportDeclaration).not.toBeNull();
2138+
expect(exportDeclaration!.node).toBe(bundle.file);
2139+
});
2140+
2141+
it('should return the source file as the declaration of "exports" in the LHS of a property access inside a local function',
2142+
() => {
2143+
loadTestFiles([EXPORTS_IDENTIFIERS_FILE]);
2144+
const bundle = makeTestBundleProgram(EXPORTS_IDENTIFIERS_FILE.name);
2145+
const host = createHost(bundle, new UmdReflectionHost(new MockLogger(), false, bundle));
2146+
const simpleFn = getDeclaration(
2147+
bundle.program, EXPORTS_IDENTIFIERS_FILE.name, 'simpleFn', ts.isFunctionDeclaration);
2148+
const exportsStatement = walkForNode(simpleFn, isExportsStatement);
2149+
if (exportsStatement === undefined) {
2150+
throw new Error('Bad test - unable to find `exports.foo = 42;` statement');
2151+
}
2152+
const exportsIdentifier = exportsStatement.expression.left.expression;
2153+
const exportDeclaration = host.getDeclarationOfIdentifier(exportsIdentifier);
2154+
expect(exportDeclaration).not.toBeNull();
2155+
expect(exportDeclaration!.node).toBe(bundle.file);
2156+
});
2157+
2158+
it('should return the variable declaration if a standalone "exports" is declared locally',
2159+
() => {
2160+
loadTestFiles([EXPORTS_IDENTIFIERS_FILE]);
2161+
const bundle = makeTestBundleProgram(EXPORTS_IDENTIFIERS_FILE.name);
2162+
const host = createHost(bundle, new UmdReflectionHost(new MockLogger(), false, bundle));
2163+
const localVarFn = getDeclaration(
2164+
bundle.program, EXPORTS_IDENTIFIERS_FILE.name, 'localVar', ts.isFunctionDeclaration);
2165+
const exportsVar = walkForNode(localVarFn, ts.isVariableDeclaration);
2166+
if (exportsVar === undefined) {
2167+
throw new Error('Bad test - unable to find `var exports = {}` statement');
2168+
}
2169+
const exportsIdentifier = exportsVar.name;
2170+
if (exportsIdentifier === undefined || !ts.isIdentifier(exportsIdentifier)) {
2171+
throw new Error('Bad test - unable to find `var exports = {};` statement');
2172+
}
2173+
const exportsDeclaration = host.getDeclarationOfIdentifier(exportsIdentifier);
2174+
expect(exportsDeclaration).not.toBeNull();
2175+
expect(exportsDeclaration!.node).toBe(exportsVar);
2176+
});
2177+
2178+
it('should return the variable declaration of "exports" in the LHS of a property access, if it is declared locally',
2179+
() => {
2180+
loadTestFiles([EXPORTS_IDENTIFIERS_FILE]);
2181+
const bundle = makeTestBundleProgram(EXPORTS_IDENTIFIERS_FILE.name);
2182+
const host = createHost(bundle, new UmdReflectionHost(new MockLogger(), false, bundle));
2183+
const localVarFn = getDeclaration(
2184+
bundle.program, EXPORTS_IDENTIFIERS_FILE.name, 'localVar', ts.isFunctionDeclaration);
2185+
const exportsVar = walkForNode(localVarFn, ts.isVariableDeclaration);
2186+
const exportsStatement = walkForNode(localVarFn, isExportsStatement);
2187+
if (exportsStatement === undefined) {
2188+
throw new Error('Bad test - unable to find `exports.foo = 42;` statement');
2189+
}
2190+
const exportsIdentifier = exportsStatement.expression.left.expression;
2191+
const exportDeclaration = host.getDeclarationOfIdentifier(exportsIdentifier);
2192+
expect(exportDeclaration).not.toBeNull();
2193+
expect(exportDeclaration?.node).toBe(exportsVar);
2194+
});
2195+
2196+
it('should return the variable declaration of "exports" in the LHS of a property access, if it is a local function parameter',
2197+
() => {
2198+
loadTestFiles([EXPORTS_IDENTIFIERS_FILE]);
2199+
const bundle = makeTestBundleProgram(EXPORTS_IDENTIFIERS_FILE.name);
2200+
const host = createHost(bundle, new UmdReflectionHost(new MockLogger(), false, bundle));
2201+
const exportsArgFn = getDeclaration(
2202+
bundle.program, EXPORTS_IDENTIFIERS_FILE.name, 'exportsArg',
2203+
ts.isFunctionDeclaration);
2204+
const exportsVar = exportsArgFn.parameters[0];
2205+
const exportsStatement = walkForNode(exportsArgFn, isExportsStatement);
2206+
if (exportsStatement === undefined) {
2207+
throw new Error('Bad test - unable to find `exports.foo = 42;` statement');
2208+
}
2209+
const exportsIdentifier = exportsStatement.expression.left.expression;
2210+
const exportDeclaration = host.getDeclarationOfIdentifier(exportsIdentifier);
2211+
expect(exportDeclaration).not.toBeNull();
2212+
expect(exportDeclaration?.node).toBe(exportsVar);
2213+
});
2214+
20812215
it('should recognize TypeScript helpers (as function declarations)', () => {
20822216
const file: TestFile = {
20832217
name: _('/test.js'),
@@ -3232,3 +3366,8 @@ runInEachFileSystem(() => {
32323366
});
32333367
});
32343368
});
3369+
3370+
type WalkerPredicate<T extends ts.Node> = (node: ts.Node) => node is T;
3371+
function walkForNode<T extends ts.Node>(node: ts.Node, predicate: WalkerPredicate<T>): T|undefined {
3372+
return node.forEachChild(child => predicate(child) ? child : walkForNode(child, predicate));
3373+
}

0 commit comments

Comments
 (0)