Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(compiler-cli): Support resolve animation name from the DTS #45107

Closed

Conversation

ivanwonder
Copy link
Contributor

Before this, the compiler resolves the value in the DTS as dynamic.
If the trigger is imported from @angular/animations, this PR will
use FFR to simulate the actual implementation in JS and extracts the
animation name.

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • angular.io application / infrastructure changes
  • Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

@ivanwonder ivanwonder force-pushed the resolve-animation-name-from-dts branch 2 times, most recently from cce1dd7 to 9513a15 Compare February 16, 2022 15:59
@jessicajaniuk jessicajaniuk added the area: compiler Issues related to `ngc`, Angular's template compiler label Feb 16, 2022
@ngbot ngbot bot added this to the Backlog milestone Feb 16, 2022
@ivanwonder ivanwonder force-pushed the resolve-animation-name-from-dts branch from 9513a15 to 5f91105 Compare February 17, 2022 00:32
@ivanwonder ivanwonder marked this pull request as ready for review February 17, 2022 00:43
@ivanwonder ivanwonder requested a review from JoostK February 17, 2022 00:44
@ivanwonder
Copy link
Contributor Author

@atscott This is the fix for animation completion in the DTS

@atscott
Copy link
Contributor

atscott commented Feb 18, 2022

I'll leave this one for @JoostK to review since he knows how this part works much better than I do :)

@atscott atscott added the target: patch This PR is targeted for the next patch release label Feb 18, 2022
Before this, the compiler resolves the value in the DTS as dynamic.
If the `trigger` is imported from `@angular/animations`, this PR will
use FFR to simulate the actual implementation in JS and extracts the
animation name.
@ivanwonder ivanwonder force-pushed the resolve-animation-name-from-dts branch from 5f91105 to a1ff918 Compare February 19, 2022 09:57
@ivanwonder ivanwonder requested a review from JoostK February 19, 2022 10:09
Copy link
Member

@JoostK JoostK left a comment

Choose a reason for hiding this comment

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

Thanks for identifying this problem and providing a fix!

@JoostK JoostK added action: merge The PR is ready for merge by the caretaker action: presubmit The PR is in need of a google3 presubmit labels Feb 19, 2022
@ngbot

This comment was marked as resolved.

@atscott atscott added target: minor This PR is targeted for the next minor release and removed action: presubmit The PR is in need of a google3 presubmit target: patch This PR is targeted for the next patch release labels Feb 22, 2022
@atscott
Copy link
Contributor

atscott commented Feb 22, 2022

@ivanwonder This had a merge conflict with the patch branch. Could you create a new PR that targets the 13.2.x branch as well?

@atscott
Copy link
Contributor

atscott commented Feb 22, 2022

This PR was merged into the repository by commit c0778b4.

@atscott atscott closed this in c0778b4 Feb 22, 2022
@ivanwonder ivanwonder deleted the resolve-animation-name-from-dts branch February 23, 2022 00:17
@ivanwonder
Copy link
Contributor Author

@ivanwonder This had a merge conflict with the patch branch. Could you create a new PR that targets the 13.2.x branch as well?

@atscott done #45169

@angular-automatic-lock-bot
Copy link

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 Mar 26, 2022
josmar-crwdstffng pushed a commit to josmar-crwdstffng/angular that referenced this pull request Apr 8, 2022
…ar#45107)

Before this, the compiler resolves the value in the DTS as dynamic.
If the `trigger` is imported from `@angular/animations`, this PR will
use FFR to simulate the actual implementation in JS and extracts the
animation name.

PR Close angular#45107
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 target: minor This PR is targeted for the next minor release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants