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

Multiple updates for an element may run during parsing depending on script type #258

Closed
3 of 6 tasks
kevinpschaaf opened this issue Oct 10, 2018 · 1 comment · Fixed by #368
Closed
3 of 6 tasks
Assignees
Milestone

Comments

@kevinpschaaf
Copy link
Member

kevinpschaaf commented Oct 10, 2018

Description

If an element definition is loaded synchronously with a legacy script, update may be called multiple times for any main-document elements that follow the script in the page: one for any defaults set in the constructor, plus one for each attributeChangedCallback.

If an element definition is loaded using defer semantics (e.g. type="module" or defer), update is only called once for such elements.

The assumption that one microtask is enough to collect the constructor defaults and overrides via attributes appears to be insufficient in the first case above.

Live Demo

Note console logging:

  • Expected (using module script): demo, code
    • Update is called once
  • Unexpected (using bundled legacy script): demo, code
    • Update is called 4 times (ctor + 3x attributes)

It appears we need to add back the optimization (used in Polymer 2) to not flush property changes (e.g. the initial _validate -> update -> render) until the element has been connected, to ensure efficient startup of the element and avoid spamming the update/render lifecycles.

Applying the following stopgap DeferUntilConnected mixin demonstrates the expected improvements, but ideally changedProperties should accumulate until the actual update, which this does not attempt: code, demo

Browsers Affected

  • Chrome
  • Firefox
  • Edge
  • Safari 12
  • Safari 11
  • IE 11

Versions

  • lit-element: v0.6.2
@sorvell
Copy link
Member

sorvell commented Dec 13, 2018

OK, I think we've finally understood the issue here. A microtask checkpoint may be triggered as a result of element creation. This is noted in the spec at the end of the Requirements for Custom Elements Constructors section. Empirically, this appears only to occur for parser generated elements with existing definitions.

This means that any LitElement that sets a property's default value in the constructor (at least if it is generated by the parser when defined) may exhibit a couple different issues: (1) update may be called more times than expected, (2) if a reflecting property is set, there may be an error Uncaught DOMException: Failed to construct 'CustomElement': The result must not have attributes. Here's a minimal repro.

Deferring updating until connectedCallback as suggested above should fix this issue.

sorvell pushed a commit that referenced this issue Dec 13, 2018
Fixes #290. The previous private `_validate` method has been made protected and named `performUpdate`. It's all now possible to return a promise from this method and doing so will block the update's completion until the promise is resolved. This allows control over the timing of update and integration with schedulers.

Fixes #258. Updates are now deferred until element connection. This ensures that microtask checkpoints that may occur during element construction do not generate errors or cause extra update cycles.
kevinpschaaf pushed a commit that referenced this issue Dec 18, 2018
* Adds `performUpdate` and defers update until connection

Fixes #290. The previous private `_validate` method has been made protected and named `performUpdate`. It's all now possible to return a promise from this method and doing so will block the update's completion until the promise is resolved. This allows control over the timing of update and integration with schedulers.

Fixes #258. Updates are now deferred until element connection. This ensures that microtask checkpoints that may occur during element construction do not generate errors or cause extra update cycles.

* Format.

* Address review feedback.

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

Successfully merging a pull request may close this issue.

3 participants