[NO-TICKET] Maintain local state in web components when attributes ch…

…ange (#3253)

* Experimenting with not re-creating the entire render tree context for attr changes

* Use one signal for the whole props object basically

* TEMPORARY: Test it out with a controlled `ds-text-field` story

* The signal value wasn't actually changing, so no re-renders

Fixed that by making sure we create a new object every time a value changes, and I also updated the unit tests to separate two test cases that had been made into one

* More comments and cleaning up

* Get rid of the `parent` attribute, which was only there for convenience

but we haven't actually found it helpful

* Expand these comments

* Undo temporary changes to a story for testing

* Remove commented-out code

* Update DOM snapshots after removing the `parent` attribute

packages/design-system/package.json

@@ -56,6 +56,7 @@
   ],
   "dependencies": {
     "@popperjs/core": "^2.4.4",
+    "@preact/signals": "1.3.0",
     "@types/react": "^18.3.1",
     "@types/react-dom": "^18.3.0",
     "@types/react-transition-group": "^4.4.5",
@@ -69,7 +70,7 @@
     "inquirer": "^9.0.2",
     "lodash": "^4.17.21",
     "ora": "^6.1.2",
-    "preactement": "1.8.5",
+    "preact": "10.11.3",
     "prop-types": "^15.8.1",
     "react-aria": "^3.27.0",
     "react-day-picker": "8.10.1",

packages/design-system/src/components/web-components/ds-alert/__snapshots__/ds-alert.test.tsx.snap

@@ -8,7 +8,6 @@ exports[`Alert renders alert 1`] = `
     <div
       aria-labelledby="alert--1__a11y-label"
       class="ds-c-alert"
-      parent="[object HTMLElement]"
       role="region"
     >
       <svg

packages/design-system/src/components/web-components/ds-badge/__snapshots__/ds-badge.test.tsx.snap

@@ -5,7 +5,6 @@ exports[`Badge should render a default badge 1`] = `
   <ds-badge>
     <span
       class="ds-c-badge"
-      parent="[object HTMLElement]"
     >
       Foo
     </span>

packages/design-system/src/components/web-components/ds-button/__snapshots__/ds-button.test.tsx.snap

@@ -10,7 +10,6 @@ exports[`Button renders as an anchor with custom prop 1`] = `
     <a
       class="ds-c-button"
       href="/example"
-      parent="[object HTMLElement]"
       target="_blank"
     >
       Foo
@@ -25,7 +24,6 @@ exports[`Button renders as button 1`] = `
   <ds-button>
     <button
       class="ds-c-button"
-      parent="[object HTMLElement]"
       type="button"
     >
       Foo
@@ -44,7 +42,6 @@ exports[`Button renders disabled anchor correctly 1`] = `
     <a
       aria-disabled="true"
       class="ds-c-button"
-      parent="[object HTMLElement]"
       role="link"
     >
       Link button
@@ -62,7 +59,6 @@ exports[`Button renders disabled button 1`] = `
     <button
       class="ds-c-button"
       disabled=""
-      parent="[object HTMLElement]"
       type="button"
     >
       Foo

packages/design-system/src/components/web-components/ds-choice/__snapshots__/ds-choice.test.tsx.snap

@@ -19,7 +19,6 @@ exports[`Choice applies errorMessage to label 1`] = `
           class="ds-c-choice"
           id="choice--11"
           name="foo"
-          parent="[object HTMLElement]"
           type="checkbox"
           value="boo"
         />
@@ -80,7 +79,6 @@ exports[`Choice has a hint and requirementLabel 1`] = `
           class="ds-c-choice"
           id="choice--8"
           name="foo"
-          parent="[object HTMLElement]"
           type="checkbox"
           value="boo"
         />
@@ -128,7 +126,6 @@ exports[`Choice is a checkbox 1`] = `
           class="ds-c-choice"
           id="choice--2"
           name="foo"
-          parent="[object HTMLElement]"
           type="checkbox"
           value="boo"
         />
@@ -170,7 +167,6 @@ exports[`Choice is a radio 1`] = `
           class="ds-c-choice"
           id="choice--1"
           name="foo"
-          parent="[object HTMLElement]"
           type="radio"
           value="boo"
         />
@@ -216,7 +212,6 @@ exports[`Choice nested content renders checked and unchecked children appropriat
           class="ds-c-choice"
           id="choice--23"
           name="foo"
-          parent="[object HTMLElement]"
           type="checkbox"
           value="boo"
         />

packages/design-system/src/components/web-components/ds-date-field/__snapshots__/ds-date-field.test.tsx.snap

@@ -34,7 +34,6 @@ exports[`DateField Date Field with Error renders with error message 2`] = `
   id="static-id"
   inputmode="numeric"
   name="ds-date-field"
-  parent="[object HTMLElement]"
   type="text"
 />
 `;
@@ -74,7 +73,6 @@ exports[`DateField generates ids when no id is provided 1`] = `
   id="date-field--6"
   inputmode="numeric"
   name="ds-date-field"
-  parent="[object HTMLElement]"
   type="text"
 />
 `;
@@ -87,7 +85,6 @@ exports[`DateField renders without picker 1`] = `
   id="static-id"
   inputmode="numeric"
   name="ds-date-field"
-  parent="[object HTMLElement]"
   type="text"
 />
 `;

packages/design-system/src/components/web-components/ds-dropdown/__snapshots__/ds-dropdown.test.tsx.snap

@@ -66,7 +66,6 @@ exports[`Dropdown renders a dropdown 1`] = `
         aria-labelledby="dropdown--1__button-content dropdown--1__label"
         class="ds-c-dropdown__button ds-c-field"
         id="dropdown--1"
-        parent="[object HTMLElement]"
         type="button"
       >
         <span
@@ -166,7 +165,6 @@ exports[`Dropdown renders a dropdown using html options 1`] = `
         aria-labelledby="dropdown--5__button-content dropdown--5__label"
         class="ds-c-dropdown__button ds-c-field"
         id="dropdown--5"
-        parent="[object HTMLElement]"
         type="button"
       >
         <span

packages/design-system/src/components/web-components/ds-hint/__snapshots__/ds-hint.test.tsx.snap

@@ -5,7 +5,6 @@ exports[`Hint should render a default hint 1`] = `
   <ds-hint>
     <p
       class="ds-c-hint"
-      parent="[object HTMLElement]"
     >
       Foo
     </p>

packages/design-system/src/components/web-components/ds-inline-error/__snapshots__/ds-inline-error.test.tsx.snap

@@ -8,7 +8,6 @@ exports[`InlineError should render a default inline-error 1`] = `
       aria-live="assertive"
       class="ds-c-inline-error"
       id="inline-error--1"
-      parent="[object HTMLElement]"
     >
       <svg
         aria-hidden="true"

packages/design-system/src/components/web-components/ds-label/__snapshots__/ds-label.test.tsx.snap

@@ -8,7 +8,6 @@ exports[`Label should render a basic label 1`] = `
     <label
       class="ds-c-label"
       for="field-123"
-      parent="[object HTMLElement]"
     >
       Foo
     </label>

packages/design-system/src/components/web-components/ds-pagination/__snapshots__/ds-pagination.test.tsx.snap

@@ -11,7 +11,6 @@ exports[`Pagination should render component 1`] = `
       aria-labelledby="static-id__pagination-heading"
       class="ds-c-pagination"
       id="static-id"
-      parent="[object HTMLElement]"
     >
       <span
         class="ds-u-visibility--screen-reader"
@@ -126,7 +125,6 @@ exports[`Pagination with compact prop enabled should render compact variant 1`]
       aria-labelledby="static-id__pagination-heading"
       class="ds-c-pagination ds-c-pagination--compact"
       id="static-id"
-      parent="[object HTMLElement]"
     >
       <span
         class="ds-u-visibility--screen-reader"

packages/design-system/src/components/web-components/ds-text-field/__snapshots__/ds-text-field.test.tsx.snap

@@ -24,7 +24,6 @@ exports[`ds-text-field renders text-field 1`] = `
         aria-invalid="false"
         class="ds-c-field"
         id="text-field--1"
-        parent="[object HTMLElement]"
         type="text"
       />
     </div>

packages/design-system/src/components/web-components/preactement/define.test.tsx

@@ -13,6 +13,10 @@ function flushPromises() {
   return new Promise((resolve) => setTimeout(resolve, 0));
 }
 
+function sleep(ms) {
+  return new Promise((resolve) => setTimeout(resolve, ms));
+}
+
 /* -----------------------------------
  *
  * IProps
@@ -187,26 +191,40 @@ describe('define()', () => {
       expect(element.innerHTML).toEqual('');
     });
 
-    it('updates component props when attributes are changed', () => {
-      const customTitle = 'customTitle';
-      const updatedProp = 'updated!';
-      const props = { value: 'attrUpdate' };
+    it('updates component props when attributes are changed', async () => {
+      const originalTitle = 'original';
+      const updatedTitle = 'updated!';
       const html = '<button>Click here</button>';
 
       define('message-nine', () => Message, { attributes: ['custom-title'] });
 
       const element = document.createElement('message-nine');
-      element.setAttribute('custom-title', customTitle);
-      element.setAttribute('props', JSON.stringify(props));
+      element.setAttribute('custom-title', originalTitle);
       element.innerHTML = html;
       root.appendChild(element);
+      expect(root.innerHTML).toContain(`<h2>${originalTitle}</h2><em></em>${html}`);
 
-      expect(root.innerHTML).toContain(`<h2>${customTitle}</h2><em>${props.value}</em>${html}`);
+      element.setAttribute('custom-title', updatedTitle);
+      await sleep(20);
+      expect(root.innerHTML).toContain(`<h2>${updatedTitle}</h2><em></em>${html}`);
+    });
 
-      element.setAttribute('custom-title', '');
-      element.setAttribute('props', JSON.stringify({ ...props, value: updatedProp }));
+    it('updates component props when `props` attribute is changed', async () => {
+      const originalTitle = 'original';
+      const updatedTitle = 'updated!';
+      const html = '<button>Click here</button>';
+
+      define('message-nine-and-a-half', () => Message, { attributes: ['custom-title'] });
+
+      const element = document.createElement('message-nine-and-a-half');
+      element.setAttribute('props', JSON.stringify({ customTitle: originalTitle }));
+      element.innerHTML = html;
+      root.appendChild(element);
+      expect(root.innerHTML).toContain(`<h2>${originalTitle}</h2><em></em>${html}`);
 
-      expect(root.innerHTML).toContain(`<em>${updatedProp}</em>${html}`);
+      element.setAttribute('props', JSON.stringify({ customTitle: updatedTitle }));
+      await sleep(20);
+      expect(root.innerHTML).toContain(`<h2>${updatedTitle}</h2><em></em>${html}`);
     });
 
     it('wraps component in an HOC if provided', () => {

packages/design-system/src/components/web-components/preactement/define.ts

@@ -12,6 +12,7 @@ import {
 import { templateToPreactVNode } from './parse';
 import { IOptions, ComponentFunction } from './model';
 import { kebabCaseIt } from 'case-it/kebab';
+import { signal } from '@preact/signals';
 
 /**
  * Registers the provided Preact component as a custom element in the browser. It can
@@ -90,6 +91,7 @@ function createCustomElement<T>(
     __properties = {};
     __options = options;
     __mutationObserver;
+    __propsSignal;
 
     static observedAttributes = ['props', ...attributes];
 
@@ -277,12 +279,12 @@ function onAttributeChange(this: CustomElement, name: string, _original: string,
   if (name === 'props') {
     props = { ...props, ...parseJson.call(this, updated) };
   } else {
-    props[getPropKey(name)] = updated;
+    props = { ...props, [getPropKey(name)]: updated };
   }
 
   this.__properties = props;
-
-  this.renderPreactComponent();
+  // Will trigger a re-render of the `StateWrapper` component if it changed
+  this.__propsSignal.value = props;
 }
 
 /**
@@ -353,30 +355,35 @@ function renderPreactComponent(this: CustomElement) {
   // function, or we'll get an endless feedback loop of change and re-render.
   this.__mutationObserver?.disconnect();
 
+  // We use a template to parse our innerHTML and turn it into Preact Virtual DOM (vnode)
+  // Putting the original inner content into a template also allows us to keep a copy of
+  // it for future renders where context has been lost (see function documentation).
   let template: HTMLTemplateElement | undefined = [...this.childNodes].find(isTemplate);
   if (!template) {
     template = document.createElement('template');
     template.innerHTML = wrapTemplateHtml(this.innerHTML);
   }
-
   const { vnode, slots } = templateToPreactVNode(template);
 
+  // For technical reasons, our vnode needs to be wrapped in an element that didn't exist
+  // in the original innerHTML. To keep that from getting passed to the Preact component,
+  // we need to unwrap our vnode before we pass it as the `children` prop.
   const children = unwrapTemplateVNode(vnode);
 
-  // These are the props we'll pass to the Preact component
-  const props = {
-    ...this.__properties,
-    parent: this,
-    children,
-    ...slots,
-  };
+  // If we were to call this function that we're in every time a prop changed, it would
+  // clobber the internal state of our underlying Preact component. To avoid this, we're
+  // going to tie our attributes (props) to Preact Signals that we can use to trigger
+  // re-rendering a `StateWrapper` component.
+  const propsSignal = signal(this.__properties);
+  this.__propsSignal = propsSignal;
+  const StateWrapper = () => h(this.__component, { ...propsSignal.value, ...slots, children });
 
   // TODO: Clearing everything before the Preact component render only appears to be
   // necessary for the unit tests. I haven't figured out why yet.
   [...this.childNodes].forEach((childNode) => childNode.remove());
 
   // Render the Preact component to the root of this custom element
-  render(h(this.__component, props), this);
+  render(h(StateWrapper, {}), this);
 
   // The Preact render would have removed this template, so add it back in
   this.appendChild(template);

packages/design-system/src/components/web-components/preactement/shared/model.ts

@@ -1,3 +1,5 @@
+import { Signal } from '@preact/signals';
+
 /* -----------------------------------
  *
  * Component
@@ -56,6 +58,7 @@ interface CustomElement<CF = any, C = any> extends HTMLElement {
   __events?: IProps;
   __options: IOptions;
   __mutationObserver?: MutationObserver;
+  __propsSignal: Signal;
 
   renderPreactComponent(): void;
 }

yarn.lock

@@ -4628,6 +4628,18 @@
   resolved "https://registry.yarnpkg.com/@preact/signals-core/-/signals-core-1.5.1.tgz#77d375fcd03fa01ac560cd2e9b038fe10a80fb23"
   integrity sha512-dE6f+WCX5ZUDwXzUIWNMhhglmuLpqJhuy3X3xHrhZYI0Hm2LyQwOu0l9mdPiWrVNsE+Q7txOnJPgtIqHCYoBVA==
 
+"@preact/signals-core@^1.7.0":
+  version "1.8.0"
+  resolved "https://registry.yarnpkg.com/@preact/signals-core/-/signals-core-1.8.0.tgz#45ffadb81b48a298a4accd26b3f6c0140cd6dacc"
+  integrity sha512-OBvUsRZqNmjzCZXWLxkZfhcgT+Fk8DDcT/8vD6a1xhDemodyy87UJRJfASMuSD8FaAIeGgGm85ydXhm7lr4fyA==
+
+"@preact/[email protected]":
+  version "1.3.0"
+  resolved "https://registry.yarnpkg.com/@preact/signals/-/signals-1.3.0.tgz#07d0210d0f813b932255ed30e0ffadae0d64fe13"
+  integrity sha512-EOMeg42SlLS72dhoq6Vjq08havnLseWmPQ8A0YsgIAqMgWgx7V1a39+Pxo6i7SY5NwJtH4849JogFq3M67AzWg==
+  dependencies:
+    "@preact/signals-core" "^1.7.0"
+
 "@preact/signals@^1.2.1":
   version "1.2.2"
   resolved "https://registry.yarnpkg.com/@preact/signals/-/signals-1.2.2.tgz#78df53b2d7e6cdda0bb32843f12eb0418a0d82b0"
@@ -19332,11 +19344,6 @@ [email protected]:
   resolved "https://registry.yarnpkg.com/preact/-/preact-10.11.3.tgz#8a7e4ba19d3992c488b0785afcc0f8aa13c78d19"
   integrity sha512-eY93IVpod/zG3uMF22Unl8h9KkrcKIRs2EGar8hwLZZDU1lkjph303V9HZBwufh2s736U6VXuhD109LYqPoffg==
 
-[email protected]:
-  version "1.8.5"
-  resolved "https://registry.yarnpkg.com/preactement/-/preactement-1.8.5.tgz#bd0ecb6c14ec8103d78d7f48d1c60031dd882e8c"
-  integrity sha512-amNlPLvhOz4ZImozZb3nizziiOGAmjLPPHGgKYE0192QsPALqq/lXECyzP6twsrvdidM0ay8zGHR1rgSKJxejw==
-
 prebuild-install@^7.1.1:
   version "7.1.1"
   resolved "https://registry.yarnpkg.com/prebuild-install/-/prebuild-install-7.1.1.tgz#de97d5b34a70a0c81334fd24641f2a1702352e45"