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(ivy): Run ChangeDetection on moved views #33644

Closed
wants to merge 1 commit into from

Conversation

@mhevery
Copy link
Member

mhevery commented Nov 7, 2019

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

@mhevery mhevery requested review from pkozlowski-opensource and kara Nov 7, 2019
@ngbot ngbot bot modified the milestone: needsTriage Nov 7, 2019
@googlebot googlebot added the cla: yes label Nov 7, 2019
@mhevery mhevery force-pushed the mhevery:transplant_views branch 2 times, most recently from ec43527 to 05fd3c5 Nov 7, 2019
@AndrewKushnir

This comment has been minimized.

Copy link
Contributor

AndrewKushnir commented Nov 7, 2019

Initial set of g3 presubmits to see how this PR affects g3 targets:

@@ -1478,6 +1478,15 @@ function refreshDynamicEmbeddedViews(lView: LView) {
ngDevMode && assertDefined(embeddedTView, 'TView must be allocated');
refreshView(embeddedLView, embeddedTView, embeddedTView.template, embeddedLView[CONTEXT] !);
}
const movedViews = viewOrContainer[MOVED_VIEWS];

This comment has been minimized.

Copy link
@pkozlowski-opensource

pkozlowski-opensource Nov 7, 2019

Member

Cool, I can see that you could re-use MOVED_VIEWS that I've introduced for queries

This comment has been minimized.

Copy link
@mhevery

mhevery Nov 8, 2019

Author Member

Yes but it turns out it is not exactly what we need.

@mhevery mhevery force-pushed the mhevery:transplant_views branch 3 times, most recently from 550ded3 to 7e015df Nov 8, 2019
@mhevery mhevery force-pushed the mhevery:transplant_views branch from 7e015df to d26c302 Nov 8, 2019
@mhevery

This comment has been minimized.

Copy link
Member Author

mhevery commented Nov 8, 2019

@mhevery mhevery marked this pull request as ready for review Nov 8, 2019
@mhevery mhevery requested review from angular/fw-core as code owners Nov 8, 2019
@mhevery mhevery mentioned this pull request Nov 8, 2019
5 of 14 tasks complete
@pkozlowski-opensource

This comment has been minimized.

Copy link
Member

pkozlowski-opensource commented Nov 12, 2019

LGTM

@kara
kara approved these changes Nov 12, 2019
Copy link
Contributor

kara left a comment

LGTM, one nit

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.