Polymer / polymer

Our original Web Component library.
https://polymer-library.polymer-project.org/
BSD 3-Clause "New" or "Revised" License
22.05k stars 2.02k forks source link

Synchronous property update as a side-effect of a computed method fails to propagate changes to grandchildren #5592

Open ishermandom opened 4 years ago

ishermandom commented 4 years ago

Let me just say: I've created a fairly minimal test case, but I'm still confused by it. I think there's probably some further minification that's possible to get closer to the root cause, but I'm not quite sure what to try to strip away next. I'm very confused that the issue only affects grandchildren, and not direct children.

Description

Live Demo

https://glitch.com/edit/#!/living-pegasus?path=public/my-element.html:31:28

Steps to Reproduce

  1. Create an iron-pages element containing a child and a grandchild custom element.
  2. Bind a property on the child element
  3. Synchronously update this property as a side-effect of updating the iron-pages selected property.

Expected Results

The grandchild is able to observe changes to the bound property.

Actual Results

The child is able to observe changes, but the grandchild is not.

Browsers Affected

Versions

kevinpschaaf commented 4 years ago

This construction results in a problem similar in effect to the one in https://github.com/Polymer/polymer/issues/5590. Because getSelectedTab (running during the "propagate binding" phase of effect processing) is side-effectful, it results in a second path being added to the pending properties of the child; once the child flushes, the first path causes the binding effect to <my-grandchild>.prop to run, and the same effect for the second path gets deduped.

An easy workaround here (and an overall best practice), is to keep computing functions pure (those used in computed functions and inline in templates). If you reframe getSelectedTab as a selectedTabChanged observer (e.g. static get observers: ['selectedTabChanged(prop.selected)']) and do the this.set('prop.selectedCopy', this.prop.selected) there, you can avoid the problem since mutations in observers start a new round of effect processing: https://glitch.com/edit/#!/pattern-fuschia?path=public/my-element.html:33:8

I'll keep this open as there may possibly be a common solution between this and #5590.

Also note, iron-pages is not necessary to reproduce this issue; changing it to a <div> results in the same outcome.

ishermandom commented 4 years ago

Thanks, Kevin, that makes sense. I was wondering what was so special about iron-pages – good to know that this actually applies to any computed function, and it does make sense that it's a best practice to keep computing functions pure.

stale[bot] commented 3 years ago

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.