-
Notifications
You must be signed in to change notification settings - Fork 320
Schedules requestUpdate in initialize rather than connectedCallback
#606
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
Conversation
…ack` Fixes #594. Ensures an early access of `updateComplete` will not complete before the element has first updated (at connection time).
kevinpschaaf
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
| this._hasConnectedResolver(); | ||
| this._hasConnectedResolver = undefined; | ||
| } else { | ||
| this.requestUpdate(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Summary here is we're considering the fact that requestUpdate was called every connection a bug, since the intention of this code was only to ensure a "request if not yet requested" one time on boot-up.
If users depend on tree-state in render(), they should manually requestUpdate in connectedCallback to ensure render is called when the tree state changes.
justinfagnani
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you file a documentation bug too? The update lifecycle docs lack some detail around this, even before the change.
|
Filed #614. |
|
Small issue. The comment that was left in the |
Fixes #594. Ensures an early access of
updateCompletewill not complete before the element has first updated (at connection time).