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

Array changes don't propagate into dom-if=false #4818

Closed
1 task done
andrey-git opened this issue Aug 27, 2017 · 6 comments
Closed
1 task done

Array changes don't propagate into dom-if=false #4818

andrey-git opened this issue Aug 27, 2017 · 6 comments

Comments

@andrey-git
Copy link

Description

When array property is bound to sub-element under dom-if, then changes in the array don't propagate to sub-element if dom-ifcondition is false. Then, when dom-if condition becomes true the binding becomes out of sync.

Live Demo

http://jsbin.com/pasuwapoxo/1/edit?html,console,output

Steps to Reproduce

Example:

  1. See jsbin and repeatedly click on the red rectangle
  2. The value propagates only when dom-if is true and the visual value is out of sync with the real value printed to the log.

Browsers Affected

  • Chrome
    Didn't check others.

Versions

  • Polymer: v2.0.2
@AlphaWong
Copy link

AlphaWong commented Aug 28, 2017

It seems only happen in Polymer.Element
live-demo: http://jsbin.com/vesuzifobi/edit?html,console,output

<p>tag behaviours as your expected.

It might cause by Polymer.Element issue.

Sent from my Htc HTC U11 using FastHub

@bigfanjs
Copy link

bigfanjs commented Aug 29, 2017

Hello my friend.

I don't know if you tried this before. But there is a less performant way to solve this by setting the restamp property to true which entirly removes the stamped content from and re-create and add them back to the DOM.

<template is="dom-if" if="[[ b ]]" restamp="true">
   <inner-element prop="[[ p ]]"></inner-element>
</template>

@andrey-git
Copy link
Author

Yes, this will work.

In my code I switched to just setting display: none instead of using dom-if, but a bug is still worth reporting :)

@bigfanjs
Copy link

Well done! And yes you're right ^__^

@sorvell
Copy link
Contributor

sorvell commented Aug 31, 2017

That dom-if does not update bindings when the if is false is by design. This was a requested enhancement that was added in Polymer 2.x.

When the if is true, we "sync" any values that need to be updated. Unfortunately, when an object is passed like this, it dirty checks and does not update. There are currently 2 workarounds for this: (1) use restamp on the dom-if causing it to re-render when the if becomes true or (2) use the Polymer.MutableData mixin on the element that should update (MyInnerElement in this case).

We'll leave this open since we'd like to investigate a better way to address this issue.

@sorvell
Copy link
Contributor

sorvell commented Feb 21, 2018

After consideration, we've decided we cannot fix the core issue here since it doing so requires modification of Polymer.PropertyEffect's basic data propagation mechanism and this would be too much of a breaking change.

As noted, restamp="true" can use used to avoid the issue here.

However, we recommend using immutable data patterns to have more control over how data propagates between elements. We plan to provide some helpers around working with immutable data, see: #5126.

@sorvell sorvell closed this as completed Feb 21, 2018
kevinpschaaf added a commit that referenced this issue Feb 13, 2020
* 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`.
aarongable pushed a commit to chromium/chromium that referenced this issue Dec 27, 2023
https://chromium-review.googlesource.com/c/chromium/src/+/5014869 fixed
an issue with the logic to show the settings basicPage. As a result the
dom-if for basicPage is now false when the current route is not set to
a route in the basic page (e.g. chrome://settings/reset).

Due to the way Polymer data bindings work inside dom-ifs, this meant
that prefs subproperty changes no longer triggered observers to fire,
so when settings like the theme were reset from
chrome://settings/reset, these changes were not correctly reflected in
the DOM even after the basic page dom-if flipped back to true (e.g.
after navigating to the appearance settings).

See Polymer/polymer#4818 for additional
context on this Polymer behavior.

Fixing by using a restamp on the dom-if to force a rerender when it
changes from false to true.

Bug: 1514328
Change-Id: I265ee6a992fe75027e03189dbf8eecbc63b572ff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5150227
Reviewed-by: John Lee <[email protected]>
Commit-Queue: Rebekah Potter <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1241080}
aarongable pushed a commit to chromium/chromium that referenced this issue Jan 18, 2024
…ate [M121]

https://chromium-review.googlesource.com/c/chromium/src/+/5014869 fixed
an issue with the logic to show the settings basicPage. As a result the
dom-if for basicPage is now false when the current route is not set to
a route in the basic page (e.g. chrome://settings/reset).

Due to the way Polymer data bindings work inside dom-ifs, this meant
that prefs subproperty changes no longer triggered observers to fire,
so when settings like the theme were reset from
chrome://settings/reset, these changes were not correctly reflected in
the DOM even after the basic page dom-if flipped back to true (e.g.
after navigating to the appearance settings).

See Polymer/polymer#4818 for additional
context on this Polymer behavior.

Fixing by using a restamp on the dom-if to force a rerender when it
changes from false to true.

(cherry picked from commit 418c683)

Bug: 1514328
Change-Id: I265ee6a992fe75027e03189dbf8eecbc63b572ff
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5150227
Reviewed-by: John Lee <[email protected]>
Commit-Queue: Rebekah Potter <[email protected]>
Cr-Original-Commit-Position: refs/heads/main@{#1241080}
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5202208
Reviewed-by: Demetrios Papadopoulos <[email protected]>
Cr-Commit-Position: refs/branch-heads/6167@{#1469}
Cr-Branched-From: 222e786-refs/heads/main@{#1233107}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants