Skip to content

fix(ivy): ngcc - render namespaced imported decorators correctly - #31426

Closed
petebacondarwin wants to merge 1 commit into
angular:masterfrom
petebacondarwin:ngcc-broken-umd-issue-31394
Closed

petebacondarwin wants to merge 1 commit into
angular:masterfrom
petebacondarwin:ngcc-broken-umd-issue-31394

Conversation

@petebacondarwin

Copy link
Copy Markdown
Contributor

The support for decorators that were imported via a namespace,
e.g. import * as core from @angular/core` was implemented
piecemeal. This meant that it was easy to miss situations where
a decorator identifier needed to be handled as a namepsaced
import rather than a direct import.

One such issue was that UMD processing of decorators was not
correct: the namespace was being omitted from references to
decorators.

Now the types have been modified to make it clear that a
Decorator.identifier could hold a namespaced identifier,
and the corresponding code that uses these types has been
fixed.

Fixes #31394

The support for decorators that were imported via a namespace,
e.g. `import * as core from `@angular/core` was implemented
piecemeal. This meant that it was easy to miss situations where
a decorator identifier needed to be handled as a namepsaced
import rather than a direct import.

One such issue was that UMD processing of decorators was not
correct: the namespace was being omitted from references to
decorators.

Now the types have been modified to make it clear that a
`Decorator.identifier` could hold a namespaced identifier,
and the corresponding code that uses these types has been
fixed.

Fixes angular#31394
@petebacondarwin petebacondarwin added type: bug/fix action: review The PR is still awaiting reviews from at least one requested reviewer effort1: hours hotlist: components team Related to Angular CDK or Angular Material freq2: medium target: major This PR is targeted for the next major release comp: ivy risk: medium area: compiler Issues related to `ngc`, Angular's template compiler labels Jul 5, 2019
@petebacondarwin
petebacondarwin requested review from a team July 5, 2019 10:20
@ngbot ngbot Bot added this to the Backlog milestone Jul 5, 2019

@gkalpak gkalpak left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@alexeagle alexeagle added target: patch This PR is targeted for the next patch release and removed target: major This PR is targeted for the next major release labels Jul 16, 2019
@alexeagle alexeagle added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jul 17, 2019

@kara kara left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kara kara added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Jul 18, 2019
@kara

kara commented Jul 18, 2019

Copy link
Copy Markdown
Contributor

merge-assistance: global approval

@mhevery mhevery closed this in dd664f6 Jul 18, 2019
mhevery pushed a commit that referenced this pull request Jul 18, 2019
)

The support for decorators that were imported via a namespace,
e.g. `import * as core from `@angular/core` was implemented
piecemeal. This meant that it was easy to miss situations where
a decorator identifier needed to be handled as a namepsaced
import rather than a direct import.

One such issue was that UMD processing of decorators was not
correct: the namespace was being omitted from references to
decorators.

Now the types have been modified to make it clear that a
`Decorator.identifier` could hold a namespaced identifier,
and the corresponding code that uses these types has been
fixed.

Fixes #31394

PR Close #31426
JoostK added a commit to JoostK/angular that referenced this pull request Jul 27, 2019
In angular#31426 a fix was implemented to render namespaced decorator imports
correctly, however it turns out that the fix only worked when decorator
information was extracted from static properties, not when using
`__decorate` calls.

This commit fixes the issue by creating the decorator metadata with the
full decorator expression, instead of only its name.

Closes angular#31394
sabeersulaiman pushed a commit to sabeersulaiman/angular that referenced this pull request Sep 6, 2019
…ular#31426)

The support for decorators that were imported via a namespace,
e.g. `import * as core from `@angular/core` was implemented
piecemeal. This meant that it was easy to miss situations where
a decorator identifier needed to be handled as a namepsaced
import rather than a direct import.

One such issue was that UMD processing of decorators was not
correct: the namespace was being omitted from references to
decorators.

Now the types have been modified to make it clear that a
`Decorator.identifier` could hold a namespaced identifier,
and the corresponding code that uses these types has been
fixed.

Fixes angular#31394

PR Close angular#31426
sabeersulaiman pushed a commit to sabeersulaiman/angular that referenced this pull request Sep 6, 2019
…tly (angular#31614)

In angular#31426 a fix was implemented to render namespaced decorator imports
correctly, however it turns out that the fix only worked when decorator
information was extracted from static properties, not when using
`__decorate` calls.

This commit fixes the issue by creating the decorator metadata with the
full decorator expression, instead of only its name.

Closes angular#31394

PR Close angular#31614
@angular-automatic-lock-bot

Copy link
Copy Markdown

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 15, 2019
@petebacondarwin
petebacondarwin deleted the ngcc-broken-umd-issue-31394 branch September 8, 2021 09:42
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

action: merge The PR is ready for merge by the caretaker area: compiler Issues related to `ngc`, Angular's template compiler cla: yes effort1: hours freq2: medium hotlist: components team Related to Angular CDK or Angular Material merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note risk: medium target: patch This PR is targeted for the next patch release type: bug/fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ivy-ngcc doesn't transform UMD bundles correctly

6 participants