fix(ngcc): support inline exports.<name> = <decl> style exports in UMD files#38959
Closed
petebacondarwin wants to merge 17 commits intoangular:masterfrom
Closed
fix(ngcc): support inline exports.<name> = <decl> style exports in UMD files#38959petebacondarwin wants to merge 17 commits intoangular:masterfrom
exports.<name> = <decl> style exports in UMD files#38959petebacondarwin wants to merge 17 commits intoangular:masterfrom
Conversation
JoostK
reviewed
Sep 23, 2020
5c54067 to
114a530
Compare
8fde932 to
a5e8915
Compare
alxhub
reviewed
Sep 29, 2020
83ccf8f to
8a4280b
Compare
exports to the current module in UMD filesexports.<name> = <decl> style exports in UMD files
8a4280b to
814acc5
Compare
Contributor
|
Note: removing the merge label as this was found to break an internal build (pinged @petebacondarwin directly with details) |
atscott
pushed a commit
that referenced
this pull request
Oct 12, 2020
…onReference()` (#38959) There is no need to check that the `ref.node` is of any particular type because immediately after this check the entry is tested to see if it passes `isClassDeclarationReference()`. The only difference is that the error that is reported is slightly different in the case that it is a `ref` but not one of the TS node types. Previously: ``` `Value at position ${idx} in the NgModule.${arrayName} of ${ className} is not a reference` ``` now ``` `Value at position ${idx} in the NgModule.${arrayName} of ${ className} is not a class` ``` Arguably the previous message was wrong, since this entry IS a reference but is not a class. PR Close #38959
atscott
pushed a commit
that referenced
this pull request
Oct 12, 2020
The `isAssignment` and `isAssignmentStatement` are not used in this file. PR Close #38959
atscott
pushed a commit
that referenced
this pull request
Oct 12, 2020
This clarifies that this is specifically about statements of the form
`exports.<name> = <declaration>`, rather than a general export
statement such as `export class <ClassName> { ... }`.
PR Close #38959
atscott
pushed a commit
that referenced
this pull request
Oct 12, 2020
The `SIMPLE_CLASS_FILE` contained a `ChildClass` that had an internal aliases implementation and extended a `SuperClass` base class. The call to `__extends` was using the wrong argument for the child class. PR Close #38959
atscott
pushed a commit
that referenced
this pull request
Oct 12, 2020
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
atscott
pushed a commit
that referenced
this pull request
Oct 12, 2020
Sometimes UMD exports appear in the following form: ``` exports.MyClass = alias1 = alias2 = <<declaration>> ``` Previously the declaration of the export would have been captured as `alias1 = alias2 = <<declaration>>`, which the `PartialInterpreter` would have failed on, since it cannot handle assignments. Now we skip over these aliases capturing only the `<<declaration>>` expression. Fixes #38947 PR Close #38959
atscott
pushed a commit
that referenced
this pull request
Oct 12, 2020
…t to code format (#38959) Previously `getDeclaration()` would only return the first node that matched the name passed in and then assert the predicate on this single node. It also only considered a subset of possible declaration types that we might care about. Now the function will parse the whole tree collecting an array of all the nodes that match the name. It then filters this array based on the predicate and only errors if the filtered array is empty. This makes this function much more resilient to more esoteric code formats such as UMD. PR Close #38959
atscott
pushed a commit
that referenced
this pull request
Oct 12, 2020
This makes these tests more resilient to changes in the test code structure. For example switching from ``` var SomeClass = <implementation>; exports.SomeClass = SomeClass; ``` to ``` exports.SomeClass = <implementation>; ``` PR Close #38959
atscott
pushed a commit
that referenced
this pull request
Oct 12, 2020
Previously the `ConcreteDeclaration` and `InlineDeclaration` had different properties for the underlying node type. And the `InlineDeclaration` did not store a value that represented its declaration. It turns out that a natural declaration node for an inline type is the expression. For example in UMD/CommonJS this would be the `exports.<name>` property access node. So this expression is now used for the `node` of `InlineDeclaration` types and the `expression` property is dropped. To support this the codebase has been refactored to use a new `DeclarationNode` type which is a union of `ts.Declaration|ts.Expression` instead of `ts.Declaration` throughout. PR Close #38959
atscott
pushed a commit
that referenced
this pull request
Oct 12, 2020
Previously these tests were checking multiple specific expression types. The new helper function is more general and will also support `PropertyAccessExpression` nodes for `InlineDeclaration` types. PR Close #38959
atscott
pushed a commit
that referenced
this pull request
Oct 12, 2020
Previously, any declarations that were defined "inline" were not
recognised by the `UmdReflectionHost`.
For example, the following syntax was completely unrecognized:
```ts
var Foo_1;
exports.Foo = Foo_1 = (function() {
function Foo() {}
return Foo;
})();
exports.Foo = Foo_1 = __decorate(SomeDecorator, Foo);
```
Such inline classes were ignored and not processed by ngcc.
This lack of processing led to failures in Ivy applications that relied
on UMD formats of libraries such as `syncfusion/ej2-angular-ui-components`.
Now all known inline UMD exports are recognized and processed accordingly.
Fixes #38947
PR Close #38959
atscott
pushed a commit
that referenced
this pull request
Oct 12, 2020
Previously directive "queries" that relied upon a namespaced type
```ts
queries: {
'mcontent': new core.ContentChild('test2'),
}
```
caused an error to be thrown. This is now supported.
PR Close #38959
atscott
pushed a commit
that referenced
this pull request
Oct 12, 2020
…identity (#38959) Previously the `node.name` property was only checked to ensure it was defined. But that meant that it was a `ts.BindingName`, which also includes `ts.BindingPattern`, which we do not support. But these helper methods were forcefully casting the value to `ts.Identifier. Now we also check that the `node.name` is actually an `ts.Identifier`. PR Close #38959
petebacondarwin
added a commit
to petebacondarwin/angular
that referenced
this pull request
Oct 14, 2020
…lar#38959) The message now also reports the name of the predicate function that failed. PR Close angular#38959
petebacondarwin
added a commit
to petebacondarwin/angular
that referenced
this pull request
Oct 14, 2020
…onReference()` (angular#38959) There is no need to check that the `ref.node` is of any particular type because immediately after this check the entry is tested to see if it passes `isClassDeclarationReference()`. The only difference is that the error that is reported is slightly different in the case that it is a `ref` but not one of the TS node types. Previously: ``` `Value at position ${idx} in the NgModule.${arrayName} of ${ className} is not a reference` ``` now ``` `Value at position ${idx} in the NgModule.${arrayName} of ${ className} is not a class` ``` Arguably the previous message was wrong, since this entry IS a reference but is not a class. PR Close angular#38959
petebacondarwin
added a commit
to petebacondarwin/angular
that referenced
this pull request
Oct 14, 2020
The `isAssignment` and `isAssignmentStatement` are not used in this file. PR Close angular#38959
petebacondarwin
added a commit
to petebacondarwin/angular
that referenced
this pull request
Oct 14, 2020
…ar#38959) This clarifies that this is specifically about statements of the form `exports.<name> = <declaration>`, rather than a general export statement such as `export class <ClassName> { ... }`. PR Close angular#38959
petebacondarwin
added a commit
to petebacondarwin/angular
that referenced
this pull request
Oct 14, 2020
…ngular#38959) If the `symbol` for the given `node` does not exist then there is no point in creating the `map`. PR Close angular#38959
petebacondarwin
added a commit
to petebacondarwin/angular
that referenced
this pull request
Oct 14, 2020
The `SIMPLE_CLASS_FILE` contained a `ChildClass` that had an internal aliases implementation and extended a `SuperClass` base class. The call to `__extends` was using the wrong argument for the child class. PR Close angular#38959
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.

UMD files export values by assigning them to the
exportsvariable.When evaluating expressions ngcc was failing to cope with inline declarations
like
exports.MyComponent = <decl>.Fixes #38947