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

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

Open
ishermandom opened this issue Sep 17, 2019 · 3 comments
Labels

Comments

@ishermandom
Copy link

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

  • [ x ] Chrome
  • [ ? ] Firefox
  • [ ? ] Edge
  • [ x ] Safari 12
  • [ ? ] Safari 10
  • [ ? ] IE 11

Versions

  • Polymer: v2.0.0
  • webcomponents: v1.0.0
@kevinpschaaf
Copy link
Member

kevinpschaaf commented Sep 17, 2019

This construction results in a problem similar in effect to the one in #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
Copy link
Author

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.

@ishermandom ishermandom changed the title iron-pages fails to propagate synchronous changes to grandchildren Synchronous property update as a side-effect of a computed method fails to propagate changes to grandchildren Oct 3, 2019
@stale
Copy link

stale bot commented Oct 3, 2020

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.

@stale stale bot added the wontfix label Oct 3, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants