Polymer: Discourage using ready callback 2.x

[QuackMartins]

We should discourage usage of ready callback, and maybe... the upgrader should move code from ready() -> connectedCallback()

[arthurevans]

Why discourage the use of ready?

We should caution against the overuse of ready, perhaps (fort example, doing static config instead of responding to state change using observers, slotchange, etc.) ... But I think there's still a valid case for one-time initialization code.

[QuackMartins]

I forgot to put it's related to Polymer 2.0

So, Polymer 2 is getting closer to native api... then it makes more sense using connectedCallback or constructor

[arthurevans]

OK... Just wondering if you had a particular reason for wanting to do away with ready.

It's true that Polymer 2.0 is getting closer to the native API... But connectedCallback can be called more than once, and in the constructor, you can't read attributes or manipulate DOM. Polymer needs something like ready in order to create shadow DOM and initialize the data system. So it probably makes sense to expose it to the user for other one-time initialization.

That said, I'd love to give people good guidance for avoiding overusing ready. I defer to @sorvell and @kevinpschaaf on whether we want to discourage use of it altogether, or provide more prescriptive advice on what to put in there.

[QuackMartins]

No particular reason :)

It's just because I was upgrading my elements then I felt like ready code was not making sense anymore

I don't want to remove ready callback... this api is important... It's only about the guidance and for staying as close as possible to the native api (which poly 2.0 is doing really great by the way)

[kevinpschaaf]

@arthurevans Your answer here is spot-on.

Re: better guidance, I think it's true that there is actually very little legitimate work a user should put in ready and not elsewhere. Perhaps the docs related to lifecycle callbacks could list some more specific examples of work appropriate for each callback (they already list general guidance):

Example callback work:

• constructor
  
  
  
  • Set property defaults (idiomatically, as opposed to properties: { prop: { value: ... } }
  
  
  • Create bound listener functions (e.g. this._boundHandleEvent = this._handleEvent.bind(this);, although somewhat obviated by ES6 => functions)
  
  
  • Set up a Polymer.dom(this).observeNodes observer to watch for children
  
  
  • _Note: do not read attributes or children, or add attributes or children (forbidden by spec)_
  
  
• connectedCallback / disconnectedCallback
  
  
  
  • Add/remove event listeners to self or window/document (only if the listeners should not fire when the element is outside the document; otherwise can just on-time addEventListener in constructor or defer to requestAnimationFrame)
  
  
  • Enqueue a requestAnimatinoFrame to perform measurement based on rendered layout in DOM
  
  
  • _Note: Generally all setup work in connectedCallback should be balanced with associated teardown work in disconnectedCallback; if not it usually means connectedCallback is the wrong place to do it_
  
  
  • _Note: do not read children in connectedCallback, as they are not guaranteed to be parsed yet per the spec; instead, use the observeNodes helper to detect when children are added or distributed to this element._
  
  
• Polymer.dom(this).observeNodes called from constructor
  
  
  
  • Inspect children (e.g. read their tag name, attributes, etc.)
  
  
  • Add/remove event listeners to/from children
  
  
  • Set properties or attributes on children
  
  
• observer / observers
  
  
  
  • Inspect property values and change thereof, and perform side effects based on their value
  
  
• ready
  
  
  
  • ❔
  
  

At this point it's hard to contrive _general_ a use case where ready is the only/best place to do work, although they do come up from time to time in very specific cases. That said, as Arthur mentioned Polymer still uses it internally and we just left it publicly overridable mostly for BC.

Alternately, maybe a list of things you might be _tempted_ to do in ready that should be done elsewhere would be valuable in the docs...?

List of ready anti-patterns:

• ⛔ Inspect or modify children
  
  
  
  • Do from Polymer.dom(this).observeNodes callback instead
  
  
  • Rationale: children may not be parsed yet, since ready may be called from connectedCallback for top-level element. Also, observeNodes callback is called for _every_ change to the child nodes and changes to distributed nodes; performing one-time work means the element is not robust to children changes.
  
  
• ⛔ Perform work based on property or attribute values
  
  
  
  • Do from and observer instead
  
  
  • Rationale: ready is called only once, so work done here based on a property value may become invalidated if the property value is changed at a later point by the user
  
  
• ⛔ Measure layout or computed style information
  
  
  
  • Do from requestAnimationFrame enqueued from connectedCallback (if one-time) or an observer (if measurement depends on current property values)
  
  
  • Rationale: the ready callback may be called before the browser's style system can return proper layout/computed style information; requestAnimationFrame is a safe place to perform measurement.
  
  
• ⛔ Add event listeners to self or window/document
  
  
  
  • Do from either 1.) constructor, 2.) connectedCallback/disconnectedCallback (if listener must be removed when out of document), or 3.) Polymer.RenderStatus.afterNextRender (for better performance if listener is not required for first paint)
  
  
  • Rationale: ready is likely not superior to constructor for this use case, and connectedCallback or afterNextRender are probably even better.
  
  

@arthurevans @sorvell wdyt?

[arthurevans]

Thanks @kevinpschaaf I think that's super helpful.

Couple of questions:

• Are we going to keep recommending observeNodes as opposed to slotchange? One of use cases I saw for ready was to observe the initial distribution, which isn't reported in slotchange. (Not immediately in ready, of course, but in a rAF or afterNextRender since as far as I know, you only need to do this once, and for obvious reasons it needs to happen after the shadow tree is created.)
• When you talk about adding event listeners from afterNextRender, I take that to mean aNR enqueued from either constructor or connectedCallback (depending on whether you need to remove and re-add the listener).
• Just so I can explain this correctly... The difference between rAF and aNR here if I'm understanding correctly is that at rAF time we expect layout to be done but not paint (earliest point you could expect measurements) and aNR should be after paint?

[kevinpschaaf]

Are we going to keep recommending observeNodes as opposed to slotchange?

As long as you have a shadowRoot and <slot>, for sure should use slotchange and not observeNodes

I take that to mean aNR enqueued from either constructor or connectedCallback

Correct.

and aNR should be after paint

Correct. afterNextRender is useful for deferring work that need not block paint (e.g. adding UI event listeners or aria attributes). requestAnimationFrame would be pre-paint.

Going ahead and closing since there's a lot of info here. Let me know if you have further questions.

[shipagency]

Sorry to add to a closed issue... I have a question.
I am writing a decorator element, and I am doing:

connectedCallback () {
  super.connectedCallback()

  this._field = Polymer.FlattenedNodesObserver.getFlattenedNodes(this).find(el => el.nodeType === Node.ELEMENT_NODE)
  if (!this._field) {
    throw new Error('hot-realtime-field could not find the element to be decorated')
  }

  this._fieldIsCheckable = !!this._field._hasIronCheckedElementBehavior

  this._listenToField()
}

Is this against best practices?
Since it's a decorator, it needs to discover its first child. However, this worries me:

Note: do not read children in connectedCallback, as they are not guaranteed to be parsed yet per the spec; instead, use the observeNodes helper to detect when children are added or distributed to this element.

I am obviously reading children... so, should I do this differently?