CustomElements: upgrade runs connectedCallback on an element with a throwing constructor

[davaajav-shino]

Upgrade runs connectedCallback on an element with a throwing constructor. After executing the following code log contains 'connected'.

let element = document.createElement('a-a');
let log = [];
w.customElements.define('a-a', class extends w.HTMLElement {
  constructor() {
    throw new Error();
  }
  connectedCallback() {
    log.push('connected');
  }
});
document.body.appendChild(element);

https://html.spec.whatwg.org/#concept-upgrade-an-element
enqueueing attributeChangedCallback, connectedCallback after custom elements state check in Step 2 is not enough when the the elements state is changed in Step 9.

[kojiishi]

Agree that running connectedCallback on failed elements does not look good. @domenic @dominiccooney @rniwa ?

[rniwa]

Indeed, we should make sure we only invoke reactions on a "defined" custom element. It just doesn't make much sense to invoke callback when it failed to construct/upgrade.

[domenic]

Agreed that it should not run.

These were moved early in the algorithm by whatwg/html#1366 for the reasons explained there. Given those reasons, I am not sure there is an elegant patch here where we avoid queuing in this sort of scenario. Probably the best solution is to just insert a "is not failed" check inside "invoke custom element reactions".

Hmm. Or maybe whenever we set the state to failed, we empty the element's custom element reaction queue.

[dominiccooney]

It would be no problem to drain the queue of these failed elements.

FWIW I think there may always be elements running around whose constructors threw, though; they can be created with 'new' which calls super(), discloses 'this', and then throws. We must continue to run callbacks for them because there's no effective way to detect them, that I know of.

[domenic]

FWIW I think there may always be elements running around whose constructors threw, though; they can be created with 'new' which calls super(), discloses 'this', and then throws. We must continue to run callbacks for them because there's no effective way to detect them, that I know of.

Yeah. Such elements aren't failed; they're just elements with weird constructors, I guess. This is an inevitable consequence of us handing the constructor over to developers instead of using a createdCallback design.