Fix handling of slot property (#419)

* Fix slot handling * Add changeset
Shopify · Aug 19, 2024 · 3c6bd29 · 3c6bd29
1 parent 8c62894
commit 3c6bd29
Show file tree

Hide file tree

Showing 5 changed files with 27 additions and 27 deletions.
diff --git a/.changeset/spicy-boats-leave.md b/.changeset/spicy-boats-leave.md
@@ -0,0 +1,6 @@
+---
+'@remote-dom/polyfill': patch
+'@remote-dom/core': patch
+---
+
+Fix synchronization of `slot` property in some edge cases
diff --git a/packages/core/source/elements/RemoteElement.ts b/packages/core/source/elements/RemoteElement.ts
@@ -150,7 +150,6 @@ export function createRemoteElement<
   return RemoteElementConstructor;
 }
 
-const SLOT_PROPERTY = 'slot';
 const REMOTE_EVENTS = Symbol('remote.events');
 
 interface RemoteEventRecord {
@@ -230,14 +229,16 @@ export abstract class RemoteElement<
     }
 
     this.__finalized = true;
-    const {remoteSlots, remoteProperties, remoteMethods} = this;
+    const {slottable, remoteSlots, remoteProperties, remoteMethods} = this;
 
     // finalize any superclasses
     const SuperConstructor = Object.getPrototypeOf(
       this,
     ) as typeof RemoteElement;
 
     const observedAttributes: string[] = [];
+    if (slottable) observedAttributes.push('slot');
+
     const attributeToPropertyMap = new Map<string, string>();
     const eventToPropertyMap = new Map<string, string>();
     const remoteSlotDefinitions = new Map<
@@ -341,25 +342,6 @@ export abstract class RemoteElement<
     return this;
   }
 
-  get [SLOT_PROPERTY]() {
-    return super.slot;
-  }
-
-  set [SLOT_PROPERTY](value: string) {
-    const currentSlot = this.slot;
-    const newSlot = String(value);
-
-    if (currentSlot === newSlot) return;
-
-    super.slot = value;
-
-    if (!(this.constructor as typeof RemoteElement).slottable) {
-      return;
-    }
-
-    updateRemoteElementProperty(this, SLOT_PROPERTY, this.slot);
-  }
-
   // Just need to use these types so TS doesn’t lose track of them.
   /** @internal */
   __slots?: Slots;
@@ -427,6 +409,19 @@ export abstract class RemoteElement<
   }
 
   attributeChangedCallback(key: string, _oldValue: any, newValue: any) {
+    if (
+      key === 'slot' &&
+      (this.constructor as typeof RemoteElement).slottable
+    ) {
+      updateRemoteElementProperty(
+        this,
+        key,
+        newValue ? String(newValue) : undefined,
+      );
+
+      return;
+    }
+
     const {
       remotePropertyDefinitions,
       __attributeToPropertyMap: attributeToPropertyMap,

diff --git a/packages/polyfill/source/Element.ts b/packages/polyfill/source/Element.ts
@@ -1,4 +1,4 @@
-import {NS, SLOT, ATTRIBUTES, NamespaceURI, NodeType} from './constants.ts';
+import {NS, ATTRIBUTES, NamespaceURI, NodeType} from './constants.ts';
 import {ParentNode} from './ParentNode.ts';
 import {NamedNodeMap} from './NamedNodeMap.ts';
 import {Attr} from './Attr.ts';
@@ -10,7 +10,6 @@ export class Element extends ParentNode {
   nodeType = NodeType.ELEMENT_NODE;
 
   [NS] = NamespaceURI.XHTML;
-  [SLOT] = '';
   get namespaceURI() {
     return this[NS];
   }
@@ -24,12 +23,12 @@ export class Element extends ParentNode {
   [anyProperty: string]: any;
 
   get slot() {
-    return this[SLOT];
+    return this.getAttribute('slot') ?? '';
   }
 
   set slot(slot: string) {
     const finalSlot = String(slot);
-    this[SLOT] = finalSlot;
+
     if (this.getAttribute('slot') !== finalSlot) {
       this.attributes.setNamedItem(new Attr('slot', finalSlot));
     }

diff --git a/packages/polyfill/source/NamedNodeMap.ts b/packages/polyfill/source/NamedNodeMap.ts
@@ -125,6 +125,7 @@ export class NamedNodeMap {
         attr[NS],
       );
     }
+
     return old;
   }
 
@@ -150,7 +151,7 @@ function updateElementAttribute(
   const {observedAttributes} = element.constructor as typeof Element;
   const {attributeChangedCallback} = element;
 
-  if (name === 'slot' && element.slot !== newValue) {
+  if (name === 'slot') {
     element.slot = newValue ?? '';
   }
 

diff --git a/packages/polyfill/source/constants.ts b/packages/polyfill/source/constants.ts
@@ -13,7 +13,6 @@ export const USER_PROPERTIES = Symbol('user_properties');
 export const LISTENERS = Symbol('listeners');
 export const IS_TRUSTED = Symbol('isTrusted');
 export const CONTENT = Symbol('content');
-export const SLOT = Symbol('slot');
 export const HOOKS = Symbol('hooks');
 
 // @TODO remove explicit values