Skip to content

Commit

Permalink
Setup listeners before initial render.
Browse files Browse the repository at this point in the history
Previously, “initializeHost” was called ahead of “addListeners”. There
was an edge case where such depth-first initialization made it possible
for synchronous events from children to be emitted before the delegated
event listeners had been setup on the parent.

This change ensures the following happens in order on first connection:
1. The host itself is initialized.
2. The static listeners are added.
3. The host undergoes its initial render.

Previously, (2) and (3) were conceptually swapped.
  • Loading branch information
theengineear committed Jan 31, 2023
1 parent 1acbddb commit 9eeb907
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 5 deletions.
32 changes: 31 additions & 1 deletion test/test-listeners.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,26 @@
import XElement from '../x-element.js';
import { assert, it } from '../../../@netflix/x-test/x-test.js';

class TestElementChild extends HTMLElement {
connectedCallback() {
const eventType = 'connected';
const eventData = { bubbles: true, composed: true };
this.dispatchEvent(new CustomEvent(eventType, eventData));
}
}
customElements.define('test-element-child', TestElementChild);

class TestElement extends XElement {
static get properties() {
return {
clicks: {
type: Number,
default: 0,
},
connections: {
type: Number,
default: 0,
},
count: {
type: Number,
default: 0,
Expand All @@ -23,7 +36,7 @@ class TestElement extends XElement {
};
}
static get listeners() {
return { click: this.onClick };
return { click: this.onClick, connected: this.onConnected };
}
static template(html) {
return ({ clicks, count }) => {
Expand All @@ -32,6 +45,7 @@ class TestElement extends XElement {
<button id="decrement" type="button">-</button>
<span>clicks: ${clicks} count ${count}</span>
<div id="custom-event-emitter"></div>
<test-element-child id="connected-event-emitter"></test-element-child>
`;
};
}
Expand All @@ -43,6 +57,9 @@ class TestElement extends XElement {
host.count--;
}
}
static onConnected(host) {
host.connections++;
}
static onCustomEventOnce(host) {
if (this === TestElement && host.constructor === TestElement) {
host.customEventOnceCount++;
Expand Down Expand Up @@ -119,6 +136,19 @@ it('test manual lifecycle', () => {
assert(count === 2, 'listener was removed');
});

it('test synchronous event handling', () => {
// This is subtle, but it tests that if child elements emit events
// synchronously in their first render, delegated event listening from the
// parent will still work.
let count = 0;
const documentListener = () => count++;
document.addEventListener('connected', documentListener);
const el = document.createElement('test-element');
document.body.append(el);
assert(count === 1);
assert(el.connections === count);
});

it('throws for bad element on listen', () => {
const el = document.createElement('test-element');
document.body.append(el);
Expand Down
20 changes: 16 additions & 4 deletions x-element.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,7 @@ export default class XElement extends HTMLElement {

/** Extends HTMLElement.prototype.connectedCallback. */
connectedCallback() {
XElement.#initializeHost(this);
XElement.#addListeners(this);
XElement.#connectHost(this);
}

/** Extends HTMLElement.prototype.attributeChangedCallback. */
Expand All @@ -135,7 +134,7 @@ export default class XElement extends HTMLElement {

/** Extends HTMLElement.prototype.disconnectedCallback. */
disconnectedCallback() {
XElement.#removeListeners(this);
XElement.#disconnectHost(this);
}

/**
Expand Down Expand Up @@ -686,6 +685,18 @@ export default class XElement extends HTMLElement {
}

// Called once per-host from initial "connectedCallback".
static #connectHost(host) {
const initialized = XElement.#initializeHost(host);
XElement.#addListeners(host);
if (initialized) {
XElement.#updateHost(host);
}
}

static #disconnectHost(host) {
XElement.#removeListeners(host);
}

static #initializeHost(host) {
const hostInfo = XElement.#hosts.get(host);
const { initialized, invalidProperties } = hostInfo;
Expand All @@ -705,8 +716,9 @@ export default class XElement extends HTMLElement {
invalidProperties.add(property);
}
hostInfo.initialized = true;
XElement.#updateHost(host);
return true;
}
return false;
}

// Prevent shadowing from properties added to element instance pre-upgrade.
Expand Down

0 comments on commit 9eeb907

Please sign in to comment.