fix(core): Refresh transplanted views at insertion point only #35968
Conversation
This file contains 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
atscott
force-pushed
the
containsdirtyproposal
branch
7 times, most recently
from
2820401
to
cde7f9e
Compare
atscott
force-pushed
the
containsdirtyproposal
branch
from
cde7f9e
to
51591b0
Compare
|
presubmit - failures seem to be pre-existing by comparing to recent clean run global also seems to look good when compared to recent run I've also confirmed that this PR fixes the 3 of 4 test failures that were suspected to have been caused by transplanted view CD differences between Ivy and VE (I think one is actually a different issues after investigation). |
atscott
force-pushed
the
containsdirtyproposal
branch
3 times, most recently
from
ae78b21
to
d723859
Compare
atscott
requested review from
mhevery,
kara,
pkozlowski-opensource,
AndrewKushnir and
IgorMinar
and removed request for
IgorMinar and
AndrewKushnir
atscott
added
action: review
atscott
force-pushed
the
containsdirtyproposal
branch
2 times, most recently
from
895d6bd
to
2ba4f43
Compare
atscott
force-pushed
the
containsdirtyproposal
branch
9 times, most recently
from
f292ace
to
cc1ded8
Compare
kara
approved these changes
Apr 27, 2020
packages/core/test/acceptance/change_detection_transplanted_view_spec.ts
Outdated
Show resolved
Hide resolved
atscott
force-pushed
the
containsdirtyproposal
branch
2 times, most recently
from
828d6ab
to
7a258a7
Compare
mhevery
approved these changes
Apr 28, 2020
atscott
added
action: merge
AndrewKushnir
added
the
action: presubmit
|
FYI, I've started global presubmit (with Ivy) and will update this thread with the results as soon as I get them. |
|
FYI, global presubmit went well. @atscott could you please rebase this PR and resolve conflicts? |
AndrewKushnir
added
action: cleanup
Only refresh transplanted views at the insertion location in Ivy. Previously, Ivy would check transplanted views at both the insertion and declaration points. This is achieved by adding a marker to the insertion tree when we encounter a transplanted view that needs to be refreshed at its declaration. We use this marker as an extra indication that we still need to descend and refresh those transplanted views at their insertion locations even if the insertion view and/or its parents are not dirty. This change fixes several issues: * Transplanted views refreshed twice if both insertion and declaration are dirty. This could be an error if the insertion component changes result in data not being available to the transplanted view because it is slated to be removed. * CheckAlways transplanted views not refreshed if shielded by non-dirty OnPush (fixes angular#35400) * Transplanted views still refreshed when insertion tree is detached (fixes angular#21324)
atscott
force-pushed
the
containsdirtyproposal
branch
from
7a258a7
to
086a25e
Compare
AndrewKushnir
removed
the
action: cleanup
AndrewKushnir
pushed a commit
that referenced
this pull request
Apr 29, 2020
Only refresh transplanted views at the insertion location in Ivy. Previously, Ivy would check transplanted views at both the insertion and declaration points. This is achieved by adding a marker to the insertion tree when we encounter a transplanted view that needs to be refreshed at its declaration. We use this marker as an extra indication that we still need to descend and refresh those transplanted views at their insertion locations even if the insertion view and/or its parents are not dirty. This change fixes several issues: * Transplanted views refreshed twice if both insertion and declaration are dirty. This could be an error if the insertion component changes result in data not being available to the transplanted view because it is slated to be removed. * CheckAlways transplanted views not refreshed if shielded by non-dirty OnPush (fixes #35400) * Transplanted views still refreshed when insertion tree is detached (fixes #21324) PR Close #35968
|
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. |
profanis
pushed a commit
to profanis/angular
that referenced
this pull request
Sep 5, 2020
…r#35968) Only refresh transplanted views at the insertion location in Ivy. Previously, Ivy would check transplanted views at both the insertion and declaration points. This is achieved by adding a marker to the insertion tree when we encounter a transplanted view that needs to be refreshed at its declaration. We use this marker as an extra indication that we still need to descend and refresh those transplanted views at their insertion locations even if the insertion view and/or its parents are not dirty. This change fixes several issues: * Transplanted views refreshed twice if both insertion and declaration are dirty. This could be an error if the insertion component changes result in data not being available to the transplanted view because it is slated to be removed. * CheckAlways transplanted views not refreshed if shielded by non-dirty OnPush (fixes angular#35400) * Transplanted views still refreshed when insertion tree is detached (fixes angular#21324) PR Close angular#35968
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.
Only refresh transplanted views at the insertion location in Ivy.
This change fixes several issues:
are dirty. This could be an error if the insertion component changes
result in data not being available to the transplanted view because it
is slated to be removed.
non-dirty OnPush (fixes Ivy: ChangeDetection not running on transplanted views #35400)
(fixes Projected content checked, even if component is detached #21324)