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

[0.8-preview] Boolean attribute change handlers are called before localDom or lightDom are available. #1131

Closed
cdata opened this issue Jan 27, 2015 · 6 comments
Assignees

Comments

@cdata
Copy link
Contributor

cdata commented Jan 27, 2015

Considering the following contrived element built on 0.8-preview branch:

<script>
Polymer({
  is: 'simple-element',
  published: {
    awesome: Boolean
  },
  bind: {
    awesome: 'awesomeChanged'
  },
  ready: function () {
    console.log('I am ready.');
  },
  awesomeChanged: function () {
    if (this.lightDom && this.localDom) {
      console.log('I has a DOM');
    } else {
      console.log('I do not has DOM');
    }
  }
});
</script>

The first time awesomeChanged is called will happen before this.lightDom and this.localDom have been initialized on the element. It will also be called before the ready lifecycle handler has been called, which may or may not be intentional, but seems undesirable on the surface.

@cdata
Copy link
Contributor Author

cdata commented Jan 31, 2015

@sorvell Related to your configure implementation: the above described behavior is a little awkward because *Changed handlers will get called as defaults are assigned from the set of values yielded by configure. In order to work around this, one would have to measure the initialization state of other self-properties before taking action in a *Changed handler that references those properties (particularly if those properties are expected to be Object or Array types).

@arthurevans
Copy link

Could we just suppress change notifications that occur before ready? I know
on 0.5 people get confused by multiple change events, or values that get
bound before they're assigned (not sure if that's one issue or two).
On Jan 30, 2015 9:31 PM, "Christopher Joel" [email protected]
wrote:

@sorvell https://github.com/sorvell Related to your configure
implementation: the above described behavior is a little awkward because
*Changed handlers will get called as defaults are assigned from the set
of values yielded by configure. In order to work around this, one would
have to measure the initialization state of other self-properties before
taking action in a *Changed handler that references those properties
(particularly if those properties are Object or Array types).


Reply to this email directly or view it on GitHub
#1131 (comment).

@addyosmani
Copy link
Member

The first time awesomeChanged is called will happen before this.lightDom and this.localDom have been initialized on the element. It will also be called before the ready lifecycle handler has been called, which may or may not be intentional, but seems undesirable on the surface.

Would a scheduler that queued up change notifications fired prior to the lightDOM, localDOM and ready methods be desirable here? It could fire through the queue once they have finished initializing. I'd otherwise personally find notification suppression a little hard to debug.

@cdata
Copy link
Contributor Author

cdata commented Feb 1, 2015

Since configure is intended to specify defaults (to my understanding), setting properties with the "configured" default should never trigger a *Changed handler. To me, "changed" implies that there has been a change from the initialized state. If that is the intended meaning, it may be the case that these handlers are bound too early in the lifecycle.

@sorvell sorvell added 0.8 and removed 0.8 labels Mar 5, 2015
@sorvell
Copy link
Contributor

sorvell commented Mar 5, 2015

lightDom and localDom have been removed via 7c7e426.

@sorvell
Copy link
Contributor

sorvell commented Mar 5, 2015

Configure is functioning according to the current design. Let's open a separate issue to discuss that if necessary. Closing this one.

@sorvell sorvell closed this as completed Mar 5, 2015
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

5 participants