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

Overridden computed property functions do not correctly observe dependency properties #5468

Closed
2 of 6 tasks
n1ywb opened this issue Jan 15, 2019 · 3 comments
Closed
2 of 6 tasks

Comments

@n1ywb
Copy link

n1ywb commented Jan 15, 2019

Description

When a behavior overrides a computed property from an earlier behavior, the correct computing function is called, but the arguments are incorrect, it looks like it's being called with the args for the first definition of the computed function.

Live Demo

https://github.com/ReadingPlus/polymer-computed-bug

Steps to Reproduce

  1. clone above repo
  2. polymer install
  3. polymer test

Expected Results

Tests should pass

Actual Results

Test fails because computed function called with wrong args

Browsers Affected

  • Chrome
  • Firefox
  • Edge
  • Safari 11
  • Safari 10
  • IE 11

Versions

  • Polymer: v2.7.0
  • webcomponents: 1.3.0

This does NOT affect polymer 1.0. I didn't put together a polymer 1 test case because I was trying to upgrade my app from polymer 1 to 2 when I ran into this, so trust me, it works on Polymer 1. Should be easy enough to adapt this case to polymer 1 to prove it.

@kevinpschaaf
Copy link
Member

This was a subtle but intentional change in Polymer 2.x, and is working as intended. See these notes in the API docs.

When we implemented Polymer 2, which has support for element subclassing, we took a harder look at what it meant for subclasses to override property metadata, and tightened up some of the rules to prevent confusing and unintentional side-effects.
This code was specifically added to prevent re-defining a cmoputed property: https://github.com/Polymer/polymer/blob/master/lib/mixins/element-mixin.js#L215-L222

This is because once a computed effect is added on a subclass, it is not "removed" from the super class; the setter will still run the computed function defined by the super class; in Polymer 1.x, adding more computed effects on a subclass (behavior) used to run the super computed which set the computed property and ran its side effects, then run the subclass computed a new value and re-set the property with a potentially new value; further if the subclass changed the signature of the computing method but used the same name, it could be called twice with different arguments. None of that is easy to explain or justifiable behavior, leading to us to simplify the rules when adding first-class element subclass support.

We can add a warning in the cases where a property configuration is being intentionally ignored by these rules.

@n1ywb
Copy link
Author

n1ywb commented Feb 3, 2019

@kevinpschaaf Thank you for the clarification. Does the same limitation apply to property and/or complex observers? Trying to figure out how to rework our code so it's polymer 2 compatible but still polymorphic.

@kevinpschaaf
Copy link
Member

Does the same limitation apply to property and/or complex observers?

No it does not; you can add as many observers as needed and each will be invoked. Computed functions are especially fraught since their return value is always assigned to the computed property name and thus causes side effects.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants