From 65a73299b8624f31829dcd61cabe72d69ee12f0a Mon Sep 17 00:00:00 2001 From: Jake Archibald Date: Wed, 11 Sep 2024 13:21:27 +0100 Subject: [PATCH 1/7] Implement isConnected, fix connectedCallback & disconnectedCallback --- packages/polyfill/source/Attr.ts | 1 - packages/polyfill/source/Document.ts | 3 +++ packages/polyfill/source/Node.ts | 17 ++++++++------- packages/polyfill/source/ParentNode.ts | 17 +++++++++++++-- packages/polyfill/source/constants.ts | 1 + packages/polyfill/source/shared.ts | 29 +++++++++++++++++++++++++- 6 files changed, 57 insertions(+), 11 deletions(-) diff --git a/packages/polyfill/source/Attr.ts b/packages/polyfill/source/Attr.ts index b03286d5..a43d3660 100644 --- a/packages/polyfill/source/Attr.ts +++ b/packages/polyfill/source/Attr.ts @@ -17,7 +17,6 @@ export class Attr extends Node { [NEXT]: Attr | null = null; [VALUE]: string; [OWNER_ELEMENT]: Element | null = null; - isConnected = false; constructor(name: string, value: string, namespace?: NamespaceURI | null) { super(); diff --git a/packages/polyfill/source/Document.ts b/packages/polyfill/source/Document.ts index 44285ad7..688a8896 100644 --- a/packages/polyfill/source/Document.ts +++ b/packages/polyfill/source/Document.ts @@ -5,6 +5,7 @@ import { NodeType, OWNER_DOCUMENT, HOOKS, + IS_CONNECTED, } from './constants.ts'; import type {Window} from './Window.ts'; import type {Node} from './Node.ts'; @@ -28,6 +29,7 @@ export class Document extends ParentNode { head: HTMLHeadElement; documentElement: HTMLHtmlElement; defaultView: Window; + [IS_CONNECTED] = true; constructor(defaultView: Window) { super(); @@ -37,6 +39,7 @@ export class Document extends ParentNode { this.body = setupElement(new HTMLBodyElement(), this, 'body'); this.head = setupElement(new HTMLHeadElement(), this, 'head'); + this.appendChild(this.documentElement); this.documentElement.appendChild(this.head); this.documentElement.appendChild(this.body); } diff --git a/packages/polyfill/source/Node.ts b/packages/polyfill/source/Node.ts index d0d364df..22026d33 100644 --- a/packages/polyfill/source/Node.ts +++ b/packages/polyfill/source/Node.ts @@ -8,6 +8,7 @@ import { NamespaceURI, NodeType, HOOKS, + IS_CONNECTED, } from './constants.ts'; import type {Document} from './Document.ts'; import type {ParentNode} from './ParentNode.ts'; @@ -17,6 +18,7 @@ import { isParentNode, isTextNode, cloneNode, + descendants, } from './shared.ts'; export class Node extends EventTarget { @@ -28,6 +30,7 @@ export class Node extends EventTarget { [CHILD]: Node | null = null; [PREV]: Node | null = null; [NEXT]: Node | null = null; + [IS_CONNECTED] = false; protected get [HOOKS]() { return this[OWNER_DOCUMENT].defaultView[HOOKS]; @@ -45,6 +48,10 @@ export class Node extends EventTarget { return this[OWNER_DOCUMENT]; } + get isConnected() { + return this[IS_CONNECTED]; + } + isDefaultNamespace(namespace: string) { return namespace === NamespaceURI.XHTML; } @@ -113,17 +120,13 @@ export class Node extends EventTarget { get textContent(): string | null { if (isCharacterData(this)) return this.data; let text = ''; - function walk(node: Node) { + + for (const node of descendants(this)) { if (isTextNode(node)) { text += node.data; } - const child = node[CHILD]; - if (child) walk(child); - const sibling = node[NEXT]; - if (sibling) walk(sibling); } - const child = this[CHILD]; - if (child) walk(child); + return text; } diff --git a/packages/polyfill/source/ParentNode.ts b/packages/polyfill/source/ParentNode.ts index 00902400..f0fc66ba 100644 --- a/packages/polyfill/source/ParentNode.ts +++ b/packages/polyfill/source/ParentNode.ts @@ -6,11 +6,13 @@ import { OWNER_DOCUMENT, NodeType, HOOKS, + IS_CONNECTED, } from './constants.ts'; import type {Node} from './Node.ts'; import {ChildNode, toNode} from './ChildNode.ts'; import {NodeList} from './NodeList.ts'; import {querySelectorAll, querySelector} from './selectors.ts'; +import {inclusiveDescendants} from './shared.ts'; export class ParentNode extends ChildNode { readonly childNodes = new NodeList(); @@ -66,7 +68,13 @@ export class ParentNode extends ChildNode { children.splice(children.indexOf(child), 1); } - (child as any).disconnectedCallback?.(); + if (this[IS_CONNECTED]) { + for (const node of inclusiveDescendants(child)) { + node[IS_CONNECTED] = false; + (node as any).disconnectedCallback?.(); + } + } + this[HOOKS].removeChild?.(this as any, child as any, childNodesIndex); } @@ -154,7 +162,12 @@ export class ParentNode extends ChildNode { if (isElement) this.children.push(child); } - (child as any).connectedCallback?.(); this[HOOKS].insertChild?.(this as any, child as any, insertIndex); + if (this[IS_CONNECTED]) { + for (const node of inclusiveDescendants(child)) { + node[IS_CONNECTED] = true; + (node as any).connectedCallback?.(); + } + } } } diff --git a/packages/polyfill/source/constants.ts b/packages/polyfill/source/constants.ts index 69652dbf..55561faf 100644 --- a/packages/polyfill/source/constants.ts +++ b/packages/polyfill/source/constants.ts @@ -16,6 +16,7 @@ export const PATH = Symbol('path'); export const STOP_IMMEDIATE_PROPAGATION = Symbol('stop_immediate_propagation'); export const CONTENT = Symbol('content'); export const HOOKS = Symbol('hooks'); +export const IS_CONNECTED = Symbol('is_connected'); // @TODO remove explicit values export const enum NodeType { diff --git a/packages/polyfill/source/shared.ts b/packages/polyfill/source/shared.ts index 73081cba..40a07dea 100644 --- a/packages/polyfill/source/shared.ts +++ b/packages/polyfill/source/shared.ts @@ -1,4 +1,11 @@ -import {DATA, OWNER_DOCUMENT, ATTRIBUTES, NodeType} from './constants.ts'; +import { + DATA, + OWNER_DOCUMENT, + ATTRIBUTES, + NodeType, + CHILD, + NEXT, +} from './constants.ts'; import type {Document} from './Document.ts'; import type {DocumentFragment} from './DocumentFragment.ts'; import type {Node} from './Node.ts'; @@ -78,3 +85,23 @@ export function cloneNode( return cloned; } } + +export function descendants(node: Node) { + const nodes: Node[] = []; + const walk = (node: Node) => { + nodes.push(node); + const child = node[CHILD]; + if (child) walk(child); + const sibling = node[NEXT]; + if (sibling) walk(sibling); + }; + const child = node[CHILD]; + if (child) walk(child); + return nodes; +} + +export function inclusiveDescendants(node: Node) { + const nodes = descendants(node); + nodes.unshift(node); + return nodes; +} From 7634d8fc32f9b87625094fa95ca80631d59c6fdb Mon Sep 17 00:00:00 2001 From: Jake Archibald Date: Wed, 11 Sep 2024 13:23:16 +0100 Subject: [PATCH 2/7] Only call remove/insert hooks for element parents --- packages/polyfill/source/ParentNode.ts | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/polyfill/source/ParentNode.ts b/packages/polyfill/source/ParentNode.ts index f0fc66ba..0ebfcbba 100644 --- a/packages/polyfill/source/ParentNode.ts +++ b/packages/polyfill/source/ParentNode.ts @@ -75,7 +75,9 @@ export class ParentNode extends ChildNode { } } - this[HOOKS].removeChild?.(this as any, child as any, childNodesIndex); + if (this.nodeType === NodeType.ELEMENT_NODE) { + this[HOOKS].removeChild?.(this as any, child as any, childNodesIndex); + } } replaceChild(newChild: Node, oldChild: Node) { @@ -162,12 +164,15 @@ export class ParentNode extends ChildNode { if (isElement) this.children.push(child); } - this[HOOKS].insertChild?.(this as any, child as any, insertIndex); if (this[IS_CONNECTED]) { for (const node of inclusiveDescendants(child)) { node[IS_CONNECTED] = true; (node as any).connectedCallback?.(); } } + + if (this.nodeType === NodeType.ELEMENT_NODE) { + this[HOOKS].insertChild?.(this as any, child as any, insertIndex); + } } } From 325ed2d0f505e55b68f239097b783df9cd767949 Mon Sep 17 00:00:00 2001 From: Jake Archibald Date: Wed, 11 Sep 2024 13:38:16 +0100 Subject: [PATCH 3/7] Add changesets --- .changeset/fair-lobsters-pull.md | 5 +++++ .changeset/healthy-rocks-rule.md | 5 +++++ .changeset/short-cougars-sparkle.md | 5 +++++ 3 files changed, 15 insertions(+) create mode 100644 .changeset/fair-lobsters-pull.md create mode 100644 .changeset/healthy-rocks-rule.md create mode 100644 .changeset/short-cougars-sparkle.md diff --git a/.changeset/fair-lobsters-pull.md b/.changeset/fair-lobsters-pull.md new file mode 100644 index 00000000..de77d4ce --- /dev/null +++ b/.changeset/fair-lobsters-pull.md @@ -0,0 +1,5 @@ +--- +'@remote-dom/polyfill': patch +--- + +Ensure that the insert and remove hooks are only called for element parents. diff --git a/.changeset/healthy-rocks-rule.md b/.changeset/healthy-rocks-rule.md new file mode 100644 index 00000000..d9c8fe3d --- /dev/null +++ b/.changeset/healthy-rocks-rule.md @@ -0,0 +1,5 @@ +--- +'@remote-dom/polyfill': patch +--- + +Make connectedCallback and disconnectedCallback call on connect/disconnect recursively diff --git a/.changeset/short-cougars-sparkle.md b/.changeset/short-cougars-sparkle.md new file mode 100644 index 00000000..9bae4044 --- /dev/null +++ b/.changeset/short-cougars-sparkle.md @@ -0,0 +1,5 @@ +--- +'@remote-dom/polyfill': minor +--- + +Implement node.isConnected From aea907e196f3c736f9d90c891c605dacf992c19f Mon Sep 17 00:00:00 2001 From: Jake Archibald Date: Wed, 11 Sep 2024 13:55:58 +0100 Subject: [PATCH 4/7] Ensure root is connected --- packages/core/source/tests/elements.test.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/core/source/tests/elements.test.ts b/packages/core/source/tests/elements.test.ts index c2a1fdf8..1d815d84 100644 --- a/packages/core/source/tests/elements.test.ts +++ b/packages/core/source/tests/elements.test.ts @@ -1279,6 +1279,7 @@ function createAndConnectRemoteRootElement() { const root = createRemoteRootElement(); const receiver = new TestRemoteReceiver(); root.connect(receiver.connection); + document.body.append(root); return {root, receiver}; } From 50568e55f1479f26f3d5cd886298d2c6fd69a83f Mon Sep 17 00:00:00 2001 From: Jake Archibald Date: Wed, 11 Sep 2024 14:06:11 +0100 Subject: [PATCH 5/7] Ensure root is connected --- examples/kitchen-sink/app/remote/worker/sandbox.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/kitchen-sink/app/remote/worker/sandbox.ts b/examples/kitchen-sink/app/remote/worker/sandbox.ts index 376f1c96..bd50815f 100644 --- a/examples/kitchen-sink/app/remote/worker/sandbox.ts +++ b/examples/kitchen-sink/app/remote/worker/sandbox.ts @@ -21,6 +21,7 @@ new ThreadWebWorker(self as any as Worker, { // synchronizing its children over a `RemoteConnection`. const root = document.createElement('remote-root'); root.connect(connection); + document.body.append(root); await render(root, api); }, From aab5259581ef8e1c495a57ee396b2740350a6c73 Mon Sep 17 00:00:00 2001 From: Jake Archibald Date: Wed, 11 Sep 2024 14:13:09 +0100 Subject: [PATCH 6/7] Update docs --- documentation/migrations/remote-ui-to-remote-dom.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/documentation/migrations/remote-ui-to-remote-dom.md b/documentation/migrations/remote-ui-to-remote-dom.md index 64b30453..6bee5b4a 100644 --- a/documentation/migrations/remote-ui-to-remote-dom.md +++ b/documentation/migrations/remote-ui-to-remote-dom.md @@ -159,6 +159,7 @@ icon.setAttribute('type', 'archive'); icon.setAttribute('slot', 'icon'); button.append(icon); root.append(button); +document.body.append(root); ``` ## Update React integration @@ -211,6 +212,7 @@ createRoot(remoteRoot).render(); import {createRoot} from 'react-dom/client'; const root = document.createElement('remote-root'); +document.body.append(root); createRoot(root).render(); ``` From 29a6c4df41a78741dd0443ca286e93d7b6c19f53 Mon Sep 17 00:00:00 2001 From: Jake Archibald Date: Wed, 11 Sep 2024 17:28:13 +0100 Subject: [PATCH 7/7] Rename function --- packages/polyfill/source/ParentNode.ts | 6 +++--- packages/polyfill/source/shared.ts | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/polyfill/source/ParentNode.ts b/packages/polyfill/source/ParentNode.ts index 0ebfcbba..b3aed87d 100644 --- a/packages/polyfill/source/ParentNode.ts +++ b/packages/polyfill/source/ParentNode.ts @@ -12,7 +12,7 @@ import type {Node} from './Node.ts'; import {ChildNode, toNode} from './ChildNode.ts'; import {NodeList} from './NodeList.ts'; import {querySelectorAll, querySelector} from './selectors.ts'; -import {inclusiveDescendants} from './shared.ts'; +import {selfAndDescendants} from './shared.ts'; export class ParentNode extends ChildNode { readonly childNodes = new NodeList(); @@ -69,7 +69,7 @@ export class ParentNode extends ChildNode { } if (this[IS_CONNECTED]) { - for (const node of inclusiveDescendants(child)) { + for (const node of selfAndDescendants(child)) { node[IS_CONNECTED] = false; (node as any).disconnectedCallback?.(); } @@ -165,7 +165,7 @@ export class ParentNode extends ChildNode { } if (this[IS_CONNECTED]) { - for (const node of inclusiveDescendants(child)) { + for (const node of selfAndDescendants(child)) { node[IS_CONNECTED] = true; (node as any).connectedCallback?.(); } diff --git a/packages/polyfill/source/shared.ts b/packages/polyfill/source/shared.ts index 40a07dea..587abe22 100644 --- a/packages/polyfill/source/shared.ts +++ b/packages/polyfill/source/shared.ts @@ -100,7 +100,7 @@ export function descendants(node: Node) { return nodes; } -export function inclusiveDescendants(node: Node) { +export function selfAndDescendants(node: Node) { const nodes = descendants(node); nodes.unshift(node); return nodes;