Skip to content

fix(ivy): run annotations handlers' resolve() in ngcc#28963

Closed
gkalpak wants to merge 7 commits intoangular:masterfrom
gkalpak:fix-ngcc-post-compilation
Closed

fix(ivy): run annotations handlers' resolve() in ngcc#28963
gkalpak wants to merge 7 commits intoangular:masterfrom
gkalpak:fix-ngcc-post-compilation

Conversation

@gkalpak
Copy link
Member

@gkalpak gkalpak commented Feb 25, 2019

PR Checklist

PR Type

  • Bugfix

What is the current behavior?

The resolve phase (run after all handlers have compiled) was introduced in 7d954df, but ngcc was not updated to run the handlers' resolve() methods. As a result, certain operations (such as listing directives used in component templates) would not be performed by ngcc.

What is the new behavior?

This PR fixes it by running the resolve() methods once compilation has been completed.

Does this PR introduce a breaking change?

  • Yes
  • No

Other information

Jirs issue: FW-1149

@gkalpak gkalpak requested review from a team February 25, 2019 16:53
@gkalpak gkalpak added type: bug/fix action: merge The PR is ready for merge by the caretaker effort1: hours target: major This PR is targeted for the next major release comp: ivy risk: low labels Feb 25, 2019
@ngbot ngbot bot added this to the Backlog milestone Feb 25, 2019
@gkalpak gkalpak removed the cla: yes label Feb 25, 2019
@googlebot
Copy link

☹️ Sorry, but only Googlers may change the label cla: yes.

@gkalpak gkalpak mentioned this pull request Feb 25, 2019
16 tasks
@gkalpak gkalpak added state: WIP and removed action: merge The PR is ready for merge by the caretaker labels Feb 25, 2019
Copy link
Contributor

@petebacondarwin petebacondarwin left a comment

Choose a reason for hiding this comment

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

Looks like this breaks real-life - https://circleci.com/gh/angular/angular/220656 and https://circleci.com/gh/angular/angular/220659#tests/containers/2.

Also there is a minor linting issue.

@gkalpak gkalpak force-pushed the fix-ngcc-post-compilation branch from 407c9de to 709cd89 Compare February 27, 2019 19:51
@gkalpak gkalpak force-pushed the fix-ngcc-post-compilation branch from 709cd89 to 05d5ef6 Compare March 10, 2019 09:22
@gkalpak gkalpak requested a review from a team March 10, 2019 09:22
@gkalpak gkalpak force-pushed the fix-ngcc-post-compilation branch from 05d5ef6 to 76e4716 Compare March 10, 2019 10:30
@gkalpak gkalpak added action: merge The PR is ready for merge by the caretaker and removed state: WIP labels Mar 10, 2019
@gkalpak gkalpak requested a review from petebacondarwin March 10, 2019 10:47
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps this beforeEach and the following it clauses should be put in a describe block, otherwise we are running setupAndAnalyzeProgram() twice for the tests in `describe('internal components')

Copy link
Member Author

Choose a reason for hiding this comment

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

Ha ha, yeah, I really wanted to do that, but couldn't come up with a good description 😇
I'll make something up 🤔

@gkalpak gkalpak force-pushed the fix-ngcc-post-compilation branch 3 times, most recently from 1167166 to a38c314 Compare March 16, 2019 15:09
@mary-poppins
Copy link

You can preview 407c9de at https://pr28963-407c9de.ngbuilds.io/.
You can preview 709cd89 at https://pr28963-709cd89.ngbuilds.io/.
You can preview 76e4716 at https://pr28963-76e4716.ngbuilds.io/.
You can preview 572769c at https://pr28963-572769c.ngbuilds.io/.
You can preview eda33d7 at https://pr28963-eda33d7.ngbuilds.io/.
You can preview f34fae5 at https://pr28963-f34fae5.ngbuilds.io/.
You can preview f29b4e8 at https://pr28963-f29b4e8.ngbuilds.io/.
You can preview ebcdafb at https://pr28963-ebcdafb.ngbuilds.io/.
You can preview 9720920 at https://pr28963-9720920.ngbuilds.io/.
You can preview bc6680e at https://pr28963-bc6680e.ngbuilds.io/.
You can preview 737636f at https://pr28963-737636f.ngbuilds.io/.
You can preview d41e2a8 at https://pr28963-d41e2a8.ngbuilds.io/.
You can preview 1167166 at https://pr28963-1167166.ngbuilds.io/.

@mary-poppins
Copy link

You can preview a38c314 at https://pr28963-a38c314.ngbuilds.io/.

@kara
Copy link
Contributor

kara commented Mar 18, 2019

presubmit

@kara kara added the action: presubmit The PR is in need of a google3 presubmit label Mar 18, 2019
gkalpak added 7 commits March 18, 2019 16:24
The `resolve` phase (run after all handlers have analyzed) was
introduced in 7d954df, but `ngcc` was not updated to run the handlers'
`resolve()` methods. As a result, certain operations (such as listing
directives used in component templates) would not be performed by
`ngcc`.

This commit fixes it by running the `resolve()` methods once analysis
has been completed.
@gkalpak gkalpak force-pushed the fix-ngcc-post-compilation branch from a38c314 to 778de01 Compare March 18, 2019 14:25
@mary-poppins
Copy link

You can preview 778de01 at https://pr28963-778de01.ngbuilds.io/.

@matsko matsko closed this in 4525619 Mar 18, 2019
matsko pushed a commit that referenced this pull request Mar 18, 2019
matsko pushed a commit that referenced this pull request Mar 18, 2019
matsko pushed a commit that referenced this pull request Mar 18, 2019
matsko pushed a commit that referenced this pull request Mar 18, 2019
The `resolve` phase (run after all handlers have analyzed) was
introduced in 7d954df, but `ngcc` was not updated to run the handlers'
`resolve()` methods. As a result, certain operations (such as listing
directives used in component templates) would not be performed by
`ngcc`.

This commit fixes it by running the `resolve()` methods once analysis
has been completed.

PR Close #28963
@gkalpak gkalpak deleted the fix-ngcc-post-compilation branch March 19, 2019 06:56
wKoza pushed a commit to wKoza/angular that referenced this pull request Apr 17, 2019
wKoza pushed a commit to wKoza/angular that referenced this pull request Apr 17, 2019
wKoza pushed a commit to wKoza/angular that referenced this pull request Apr 17, 2019
wKoza pushed a commit to wKoza/angular that referenced this pull request Apr 17, 2019
wKoza pushed a commit to wKoza/angular that referenced this pull request Apr 17, 2019
)

The `resolve` phase (run after all handlers have analyzed) was
introduced in 7d954df, but `ngcc` was not updated to run the handlers'
`resolve()` methods. As a result, certain operations (such as listing
directives used in component templates) would not be performed by
`ngcc`.

This commit fixes it by running the `resolve()` methods once analysis
has been completed.

PR Close angular#28963
@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 Sep 14, 2019
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 action: presubmit The PR is in need of a google3 presubmit cla: yes effort1: hours risk: low target: major This PR is targeted for the next major release type: bug/fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants