Skip to content

Commit 8d3da56

Browse files
JoostKjosephperrott
authored andcommitted
fix(ngcc): detect synthesized constructors that have been downleveled using TS 4.2 (angular#41305)
TypeScript 4.2 has changed its emitted syntax for synthetic constructors when using `downlevelIteration`, which affects ES5 bundles that have been downleveled from ES2015 bundles. This is typically the case for UMD bundles in the APF spec, as they are generated by downleveling the ESM2015 bundle into ES5. ngcc needs to detect the new syntax in order to correctly identify synthesized constructor functions in ES5 bundles. Fixes angular#41298 PR Close angular#41305
1 parent 65f7d53 commit 8d3da56

File tree

5 files changed

+429
-28
lines changed

5 files changed

+429
-28
lines changed

packages/compiler-cli/ngcc/src/host/esm5_host.ts

Lines changed: 94 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -382,6 +382,12 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost {
382382
* return _super.apply(this, tslib.__spread(arguments)) || this;
383383
* ```
384384
*
385+
* or, since TypeScript 4.2 it would be
386+
*
387+
* ```
388+
* return _super.apply(this, tslib.__spreadArray([], tslib.__read(arguments))) || this;
389+
* ```
390+
*
385391
* Such constructs can be still considered as synthetic delegate constructors as they are
386392
* the product of a common TypeScript to ES5 synthetic constructor, just being downleveled
387393
* to ES5 using `tsc`. See: https://github.com/angular/angular/issues/38453.
@@ -413,7 +419,10 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost {
413419
* ```
414420
* var _this = _super.apply(this, tslib.__spread(arguments)) || this;
415421
* ```
416-
*
422+
* or using the syntax emitted since TypeScript 4.2:
423+
* ```
424+
* return _super.apply(this, tslib.__spreadArray([], tslib.__read(arguments))) || this;
425+
* ```
417426
*
418427
* @param statement a statement that may be a synthesized super call
419428
* @returns true if the statement looks like a synthesized super call
@@ -447,6 +456,10 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost {
447456
* ```
448457
* return _super.apply(this, tslib.__spread(arguments)) || this;
449458
* ```
459+
* or using the syntax emitted since TypeScript 4.2:
460+
* ```
461+
* return _super.apply(this, tslib.__spreadArray([], tslib.__read(arguments))) || this;
462+
* ```
450463
*
451464
* @param statement a statement that may be a synthesized super call
452465
* @returns true if the statement looks like a synthesized super call
@@ -473,6 +486,10 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost {
473486
* ```
474487
* _super.apply(this, tslib.__spread(arguments)) || this;
475488
* ```
489+
* or using the syntax emitted since TypeScript 4.2:
490+
* ```
491+
* return _super.apply(this, tslib.__spreadArray([], tslib.__read(arguments))) || this;
492+
* ```
476493
*
477494
* @param expression an expression that may represent a default super call
478495
* @returns true if the expression corresponds with the above form
@@ -500,7 +517,8 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost {
500517
* This structure is generated by TypeScript when transforming ES2015 to ES5, see
501518
* https://github.com/Microsoft/TypeScript/blob/v3.2.2/src/compiler/transformers/es2015.ts#L1148-L1163
502519
*
503-
* Additionally, we also handle cases where `arguments` are wrapped by a TypeScript spread helper.
520+
* Additionally, we also handle cases where `arguments` are wrapped by a TypeScript spread
521+
* helper.
504522
* This can happen if ES2015 class output contain auto-generated constructors due to class
505523
* members. The ES2015 output will be using `super(...arguments)` to delegate to the superclass,
506524
* but once downleveled to ES5, the spread operator will be persisted through a TypeScript spread
@@ -510,6 +528,12 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost {
510528
* _super.apply(this, __spread(arguments)) || this;
511529
* ```
512530
*
531+
* or, since TypeScript 4.2 it would be
532+
*
533+
* ```
534+
* _super.apply(this, tslib.__spreadArray([], tslib.__read(arguments))) || this;
535+
* ```
536+
*
513537
* More details can be found in: https://github.com/angular/angular/issues/38453.
514538
*
515539
* @param expression an expression that may represent a default super call
@@ -538,32 +562,79 @@ export class Esm5ReflectionHost extends Esm2015ReflectionHost {
538562
// The other scenario we intend to detect: The `arguments` variable might be wrapped with the
539563
// TypeScript spread helper (either through tslib or inlined). This can happen if an explicit
540564
// delegate constructor uses `super(...arguments)` in ES2015 and is downleveled to ES5 using
541-
// `--downlevelIteration`. The output in such cases would not directly pass the function
542-
// `arguments` to the `super` call, but wrap it in a TS spread helper. The output would match
543-
// the following pattern: `super.apply(this, tslib.__spread(arguments))`. We check for such
544-
// constructs below, but perform the detection of the call expression definition as last as
545-
// that is the most expensive operation here.
546-
if (!ts.isCallExpression(argumentsExpr) || argumentsExpr.arguments.length !== 1 ||
547-
!isArgumentsIdentifier(argumentsExpr.arguments[0])) {
565+
// `--downlevelIteration`.
566+
return this.isSpreadArgumentsExpression(argumentsExpr);
567+
}
568+
569+
/**
570+
* Determines if the provided expression is one of the following call expressions:
571+
*
572+
* 1. `__spread(arguments)`
573+
* 2. `__spreadArray([], __read(arguments))`
574+
*
575+
* The tslib helpers may have been emitted inline as in the above example, or they may be read
576+
* from a namespace import.
577+
*/
578+
private isSpreadArgumentsExpression(expression: ts.Expression): boolean {
579+
const call = this.extractKnownHelperCall(expression);
580+
if (call === null) {
548581
return false;
549582
}
550583

551-
const argumentsCallExpr = argumentsExpr.expression;
552-
let argumentsCallDeclaration: Declaration|null = null;
553-
554-
// The `__spread` helper could be globally available, or accessed through a namespaced
555-
// import. Hence we support a property access here as long as it resolves to the actual
556-
// known TypeScript spread helper.
557-
if (ts.isIdentifier(argumentsCallExpr)) {
558-
argumentsCallDeclaration = this.getDeclarationOfIdentifier(argumentsCallExpr);
559-
} else if (
560-
ts.isPropertyAccessExpression(argumentsCallExpr) &&
561-
ts.isIdentifier(argumentsCallExpr.name)) {
562-
argumentsCallDeclaration = this.getDeclarationOfIdentifier(argumentsCallExpr.name);
584+
if (call.helper === KnownDeclaration.TsHelperSpread) {
585+
// `__spread(arguments)`
586+
return call.args.length === 1 && isArgumentsIdentifier(call.args[0]);
587+
} else if (call.helper === KnownDeclaration.TsHelperSpreadArray) {
588+
// `__spreadArray([], __read(arguments))`
589+
if (call.args.length !== 2) {
590+
return false;
591+
}
592+
593+
const firstArg = call.args[0];
594+
if (!ts.isArrayLiteralExpression(firstArg) || firstArg.elements.length !== 0) {
595+
return false;
596+
}
597+
598+
const secondArg = this.extractKnownHelperCall(call.args[1]);
599+
if (secondArg === null || secondArg.helper !== KnownDeclaration.TsHelperRead) {
600+
return false;
601+
}
602+
603+
return secondArg.args.length === 1 && isArgumentsIdentifier(secondArg.args[0]);
604+
} else {
605+
return false;
606+
}
607+
}
608+
609+
/**
610+
* Inspects the provided expression and determines if it corresponds with a known helper function
611+
* as receiver expression.
612+
*/
613+
private extractKnownHelperCall(expression: ts.Expression):
614+
{helper: KnownDeclaration, args: ts.NodeArray<ts.Expression>}|null {
615+
if (!ts.isCallExpression(expression)) {
616+
return null;
617+
}
618+
619+
const receiverExpr = expression.expression;
620+
621+
// The helper could be globally available, or accessed through a namespaced import. Hence we
622+
// support a property access here as long as it resolves to the actual known TypeScript helper.
623+
let receiver: Declaration|null = null;
624+
if (ts.isIdentifier(receiverExpr)) {
625+
receiver = this.getDeclarationOfIdentifier(receiverExpr);
626+
} else if (ts.isPropertyAccessExpression(receiverExpr) && ts.isIdentifier(receiverExpr.name)) {
627+
receiver = this.getDeclarationOfIdentifier(receiverExpr.name);
628+
}
629+
630+
if (receiver === null || receiver.known === null) {
631+
return null;
563632
}
564633

565-
return argumentsCallDeclaration !== null &&
566-
argumentsCallDeclaration.known === KnownDeclaration.TsHelperSpread;
634+
return {
635+
helper: receiver.known,
636+
args: expression.arguments,
637+
};
567638
}
568639
}
569640

packages/compiler-cli/ngcc/test/host/commonjs_host_spec.ts

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1530,11 +1530,15 @@ exports.MissingClass2 = MissingClass2;
15301530
break;
15311531
case 'inlined':
15321532
fileHeader =
1533-
`var __spread = (this && this.__spread) || function (...args) { /* ... */ }`;
1533+
`var __spread = (this && this.__spread) || function (...args) { /* ... */ };\n` +
1534+
`var __spreadArray = (this && this.__spreadArray) || function (...args) { /* ... */ };\n` +
1535+
`var __read = (this && this.__read) || function (...args) { /* ... */ };\n`;
15341536
break;
15351537
case 'inlined_with_suffix':
15361538
fileHeader =
1537-
`var __spread$1 = (this && this.__spread$1) || function (...args) { /* ... */ }`;
1539+
`var __spread$1 = (this && this.__spread$1) || function (...args) { /* ... */ };\n` +
1540+
`var __spreadArray$1 = (this && this.__spreadArray$1) || function (...args) { /* ... */ };\n` +
1541+
`var __read$2 = (this && this.__read$2) || function (...args) { /* ... */ };\n`;
15381542
break;
15391543
}
15401544
const file = {
@@ -1635,6 +1639,17 @@ exports.MissingClass2 = MissingClass2;
16351639
expect(parameters).toBeNull();
16361640
});
16371641

1642+
it('recognizes delegate super call using inline spreadArray helper', () => {
1643+
const parameters = getConstructorParameters(
1644+
`
1645+
function TestClass() {
1646+
return _super.apply(this, __spreadArray([], __read(arguments))) || this;
1647+
}`,
1648+
'inlined');
1649+
1650+
expect(parameters).toBeNull();
1651+
});
1652+
16381653
it('recognizes delegate super call using inline spread helper with suffix', () => {
16391654
const parameters = getConstructorParameters(
16401655
`
@@ -1646,6 +1661,17 @@ exports.MissingClass2 = MissingClass2;
16461661
expect(parameters).toBeNull();
16471662
});
16481663

1664+
it('recognizes delegate super call using inline spreadArray helper with suffix', () => {
1665+
const parameters = getConstructorParameters(
1666+
`
1667+
function TestClass() {
1668+
return _super.apply(this, __spreadArray$1([], __read$2(arguments))) || this;
1669+
}`,
1670+
'inlined_with_suffix');
1671+
1672+
expect(parameters).toBeNull();
1673+
});
1674+
16491675
it('recognizes delegate super call using imported spread helper', () => {
16501676
const parameters = getConstructorParameters(
16511677
`
@@ -1657,6 +1683,17 @@ exports.MissingClass2 = MissingClass2;
16571683
expect(parameters).toBeNull();
16581684
});
16591685

1686+
it('recognizes delegate super call using imported spreadArray helper', () => {
1687+
const parameters = getConstructorParameters(
1688+
`
1689+
function TestClass() {
1690+
return _super.apply(this, tslib.__spreadArray([], tslib.__read(arguments))) || this;
1691+
}`,
1692+
'imported');
1693+
1694+
expect(parameters).toBeNull();
1695+
});
1696+
16601697
describe('with class member assignment', () => {
16611698
it('recognizes delegate super call using inline spread helper', () => {
16621699
const parameters = getConstructorParameters(
@@ -1671,6 +1708,19 @@ exports.MissingClass2 = MissingClass2;
16711708
expect(parameters).toBeNull();
16721709
});
16731710

1711+
it('recognizes delegate super call using inline spreadArray helper', () => {
1712+
const parameters = getConstructorParameters(
1713+
`
1714+
function TestClass() {
1715+
var _this = _super.apply(this, __spreadArray([], __read(arguments))) || this;
1716+
_this.synthesizedProperty = null;
1717+
return _this;
1718+
}`,
1719+
'inlined');
1720+
1721+
expect(parameters).toBeNull();
1722+
});
1723+
16741724
it('recognizes delegate super call using inline spread helper with suffix', () => {
16751725
const parameters = getConstructorParameters(
16761726
`
@@ -1684,6 +1734,19 @@ exports.MissingClass2 = MissingClass2;
16841734
expect(parameters).toBeNull();
16851735
});
16861736

1737+
it('recognizes delegate super call using inline spreadArray helper with suffix', () => {
1738+
const parameters = getConstructorParameters(
1739+
`
1740+
function TestClass() {
1741+
var _this = _super.apply(this, __spreadArray$1([], __read$2(arguments))) || this;
1742+
_this.synthesizedProperty = null;
1743+
return _this;
1744+
}`,
1745+
'inlined_with_suffix');
1746+
1747+
expect(parameters).toBeNull();
1748+
});
1749+
16871750
it('recognizes delegate super call using imported spread helper', () => {
16881751
const parameters = getConstructorParameters(
16891752
`
@@ -1696,6 +1759,19 @@ exports.MissingClass2 = MissingClass2;
16961759

16971760
expect(parameters).toBeNull();
16981761
});
1762+
1763+
it('recognizes delegate super call using imported spreadArray helper', () => {
1764+
const parameters = getConstructorParameters(
1765+
`
1766+
function TestClass() {
1767+
var _this = _super.apply(this, tslib.__spreadArray([], tslib.__read(arguments))) || this;
1768+
_this.synthesizedProperty = null;
1769+
return _this;
1770+
}`,
1771+
'imported');
1772+
1773+
expect(parameters).toBeNull();
1774+
});
16991775
});
17001776

17011777
it('handles the case where a unique name was generated for _super or _this', () => {

0 commit comments

Comments
 (0)