Store syncInfo on instance and don't sync paths. Fixes #5629 #5630
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
* Previously the `syncInfo` used to store changed properties while the dom-if was false was stored on the dom-if. This means that even under `restamp`, any stored properties from a previous instance would be applied to the next instance. This change moves the `syncInfo` storage to the instance, so it naturally goes away when the instance is discarded. Any new instance will take current values from the host. * Due to limitations described in #4818, it is bad/illegal to allow paths to be batched and played through `runEffects`, since effect de-duping occurs by _root property_, so effects will only be running once for the first path matching an effect, with all other paths being dropped on the ground. This can be particularly bad if the user e.g. `set` a path to an object, and then subsequently nulled the object; the observer would then be acting on a path that is no longer valid. This change only stores the root property & value for any paths that come in, which matches the non-`fastDomIf` behavior with only storing `root(prop)` in `__invalidProps`.
sorvell
suggested changes
Feb 21, 2020
(same as invalidProps for non-fastDomIf)
sorvell
approved these changes
Feb 21, 2020
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
syncInfoused to store changed properties while the dom-if was false was stored on the dom-if. This means that even underrestamp, any stored properties from a previous instance would be applied to the next instance. This change moves thesyncInfostorage to the instance, so it naturally goes away when the instance is discarded. Any new instance will take current values from the host.runEffects, since effect de-duping occurs by root property, so effects will only be running once for the first path matching an effect, with all other paths being dropped on the ground. This can be particularly bad if the user e.g.seta path to an object, and then subsequently nulled the object; the observer would then be acting on a path that is no longer valid. This change only stores the root property & value for any paths that come in, which matches the non-fastDomIfbehavior with only storingroot(prop)in__invalidProps.Reference Issue
Fixes #5629