Fix parent->child fingerprint problem

Also fixes using the correct element for children always

Author: weaverryan

src/LiveComponent/CHANGELOG.md

@@ -10,6 +10,11 @@
 public User $user;
 ```
 
+-   [BC BREAK]: Child components are no longer automatically re-rendered when
+    a parent component re-renders and the value of one of the props passed to
+    the child has changed. Pass `acceptUpdatesFromParent: true` to any `LiveProp`
+    on the child component to re-enable this behavior.
+
 -   Non-persisted entity objects can now be used with `LiveProp`: it will be
     serialized using the serializer.
 

src/LiveComponent/assets/src/Backend/Backend.ts

@@ -1,12 +1,18 @@
 import BackendRequest from './BackendRequest';
 import RequestBuilder from './RequestBuilder';
 
+export interface ChildrenFingerprints {
+    // key is the id of the child component
+    [key: string]: {fingerprint: string, tag: string}
+}
+
 export interface BackendInterface {
     makeRequest(
         props: any,
         actions: BackendAction[],
         updated: {[key: string]: any},
-        childrenFingerprints: any
+        children: ChildrenFingerprints,
+        updatedPropsFromParent: {[key: string]: any},
     ): BackendRequest;
 }
 
@@ -26,13 +32,15 @@ export default class implements BackendInterface {
         props: any,
         actions: BackendAction[],
         updated: {[key: string]: any},
-        childrenFingerprints: any
+        children: ChildrenFingerprints,
+        updatedPropsFromParent: {[key: string]: any},
     ): BackendRequest {
         const { url, fetchOptions } = this.requestBuilder.buildRequest(
             props,
             actions,
             updated,
-            childrenFingerprints
+            children,
+            updatedPropsFromParent
         );
 
         return new BackendRequest(

src/LiveComponent/assets/src/Backend/RequestBuilder.ts

@@ -1,4 +1,4 @@
-import { BackendAction } from './Backend';
+import { BackendAction, ChildrenFingerprints } from './Backend';
 
 export default class {
     private url: string;
@@ -13,7 +13,8 @@ export default class {
         props: any,
         actions: BackendAction[],
         updated: {[key: string]: any},
-        childrenFingerprints: any
+        children: ChildrenFingerprints,
+        updatedPropsFromParent: {[key: string]: any},
     ): { url: string; fetchOptions: RequestInit } {
         const splitUrl = this.url.split('?');
         let [url] = splitUrl;
@@ -25,23 +26,29 @@ export default class {
             Accept: 'application/vnd.live-component+html',
         };
 
-        const hasFingerprints = Object.keys(childrenFingerprints).length > 0;
+        const hasFingerprints = Object.keys(children).length > 0;
         if (
             actions.length === 0 &&
-            this.willDataFitInUrl(JSON.stringify(props), JSON.stringify(updated), params, JSON.stringify(childrenFingerprints))
+            this.willDataFitInUrl(JSON.stringify(props), JSON.stringify(updated), params, JSON.stringify(children), JSON.stringify(updatedPropsFromParent))
         ) {
             params.set('props', JSON.stringify(props));
             params.set('updated', JSON.stringify(updated));
+            if (Object.keys(updatedPropsFromParent).length > 0) {
+                params.set('propsFromParent', JSON.stringify(updatedPropsFromParent));
+            }
             if (hasFingerprints) {
-                params.set('childrenFingerprints', JSON.stringify(childrenFingerprints));
+                params.set('children', JSON.stringify(children));
             }
             fetchOptions.method = 'GET';
         } else {
             fetchOptions.method = 'POST';
             fetchOptions.headers['Content-Type'] = 'application/json';
             const requestData: any = { props, updated };
+            if (Object.keys(updatedPropsFromParent).length > 0) {
+                requestData.propsFromParent = updatedPropsFromParent;
+            }
             if (hasFingerprints) {
-                requestData.childrenFingerprints = childrenFingerprints;
+                requestData.children = children;
             }
 
             if (actions.length > 0) {
@@ -72,8 +79,8 @@ export default class {
         }
     }
 
-    private willDataFitInUrl(propsJson: string, updatedJson: string, params: URLSearchParams, childrenFingerprintsJson: string) {
-        const urlEncodedJsonData = new URLSearchParams(propsJson + updatedJson + childrenFingerprintsJson).toString();
+    private willDataFitInUrl(propsJson: string, updatedJson: string, params: URLSearchParams, childrenJson: string, propsFromParentJson: string) {
+        const urlEncodedJsonData = new URLSearchParams(propsJson + updatedJson + childrenJson + propsFromParentJson).toString();
 
         // if the URL gets remotely close to 2000 chars, it may not fit
         return (urlEncodedJsonData + params.toString()).length < 1500;

src/LiveComponent/assets/src/Component/ValueStore.ts

@@ -20,6 +20,13 @@ export default class {
      */
     private pendingProps: {[key: string]: any} = {};
 
+    /**
+     * A list of props that the parent wants us to update.
+     *
+     * These will be sent on the next request to the server.
+     */
+    private updatedPropsFromParent: {[key: string]: any} = {};
+
     constructor(props: any) {
         this.props = props;
     }
@@ -82,6 +89,10 @@ export default class {
         return { ...this.dirtyProps };
     }
 
+    getUpdatedPropsFromParent(): any {
+        return { ...this.updatedPropsFromParent };
+    }
+
     /**
      * Called when an update request begins.
      */
@@ -95,6 +106,7 @@ export default class {
      */
     reinitializeAllProps(props: any): void {
         this.props = props;
+        this.updatedPropsFromParent = {};
         this.pendingProps = {};
     }
 
@@ -108,18 +120,19 @@ export default class {
     }
 
     /**
-     * Reinitialize *only* the provided props, but leave other untouched.
-     *
      * This is used when a parent component is rendering, and it includes
-     * a fresh set of the "readonly" props for a child. This allows any writable
-     * props to remain (as these may have already been changed by the user).
+     * a fresh set of props that should be updated on the child component.
+     *
+     * The server manages returning only the props that should be updated onto
+     * the child, so we don't need to worry about that.
      *
-     * The server manages returning only the readonly props, so we don't need to
-     * worry about that.
+     * The props are stored in a different place, because the existing props
+     * have their own checksum and these new props have *their* own checksum.
+     * So, on the next render, both need to be sent independently.
      *
-     * Returns true if any of the props changed.
+     * Returns true if any of the props are different.
      */
-    reinitializeProvidedProps(props: any): boolean {
+    storeNewPropsFromParent(props: any): boolean {
         let changed = false;
 
         for (const [key, value] of Object.entries(props)) {
@@ -129,10 +142,13 @@ export default class {
             // prop entirely
             if (currentValue !== value) {
                 changed = true;
-                this.props[key] = value;
             }
         }
 
+        if (changed) {
+            this.updatedPropsFromParent = props;
+        }
+
         return changed;
     }
 }

src/LiveComponent/assets/src/Component/index.ts

@@ -1,4 +1,4 @@
-import { BackendAction, BackendInterface } from '../Backend/Backend';
+import { BackendAction, BackendInterface, ChildrenFingerprints } from '../Backend/Backend';
 import ValueStore from './ValueStore';
 import { normalizeModelName } from '../string_utils';
 import BackendRequest from '../Backend/BackendRequest';
@@ -279,17 +279,17 @@ export default class Component {
      *
      * @param toEl
      */
-    updateFromNewElement(toEl: HTMLElement): boolean {
+    updateFromNewElementFromParentRender(toEl: HTMLElement): void {
         const props = this.elementDriver.getComponentProps(toEl);
 
         // if no props are on the element, use the existing element completely
         // this means the parent is signaling that the child does not need to be re-rendered
         if (props === null) {
-            return false;
+            return;
         }
 
         // push props directly down onto the value store
-        const isChanged = this.valueStore.reinitializeProvidedProps(props);
+        const isChanged = this.valueStore.storeNewPropsFromParent(props);
 
         const fingerprint = toEl.dataset.liveFingerprintValue;
         if (fingerprint !== undefined) {
@@ -299,8 +299,6 @@ export default class Component {
         if (isChanged) {
             this.render();
         }
-
-        return false;
     }
 
     /**
@@ -358,7 +356,8 @@ export default class Component {
             this.valueStore.getOriginalProps(),
             this.pendingActions,
             this.valueStore.getDirtyProps(),
-            this.getChildrenFingerprints()
+            this.getChildrenFingerprints(),
+            this.valueStore.getUpdatedPropsFromParent(),
         );
         this.hooks.triggerHook('loading.state:started', this.element, this.backendRequest);
 
@@ -572,16 +571,19 @@ export default class Component {
         modal.focus();
     }
 
-    private getChildrenFingerprints(): any {
-        const fingerprints: any = {};
+    private getChildrenFingerprints(): ChildrenFingerprints {
+        const fingerprints: ChildrenFingerprints = {};
 
         this.children.forEach((childComponent) => {
             const child = childComponent.component;
             if (!child.id) {
                 throw new Error('missing id');
             }
 
-            fingerprints[child.id] = child.fingerprint;
+            fingerprints[child.id] = {
+                fingerprint: child.fingerprint as string,
+                tag: child.element.tagName.toLowerCase(),
+            };
         });
 
         return fingerprints;

src/LiveComponent/assets/src/dom_utils.ts

@@ -250,19 +250,6 @@ export function htmlToElement(html: string): HTMLElement {
     return child;
 }
 
-// Inspired by https://stackoverflow.com/questions/13389751/change-tag-using-javascript
-export function cloneElementWithNewTagName(element: Element, newTag: string): HTMLElement {
-    const originalTag = element.tagName;
-    const startRX = new RegExp('^<' + originalTag, 'i');
-    const endRX = new RegExp(originalTag + '>$', 'i');
-    const startSubst = '<' + newTag;
-    const endSubst = newTag + '>';
-
-    const newHTML = element.outerHTML.replace(startRX, startSubst).replace(endRX, endSubst);
-
-    return htmlToElement(newHTML);
-}
-
 /**
  * Returns just the outer element's HTML as a string - useful for error messages.
  *

src/LiveComponent/assets/src/live_controller.ts

@@ -43,7 +43,7 @@ export default class LiveControllerDefault extends Controller<HTMLElement> imple
         listeners: { type: Array, default: [] },
         debounce: { type: Number, default: 150 },
         id: String,
-        fingerprint: String,
+        fingerprint: { type: String, default: '' },
     };
 
     declare readonly nameValue: string;

src/LiveComponent/assets/src/morphdom.ts

@@ -1,4 +1,4 @@
-import { cloneElementWithNewTagName, cloneHTMLElement, setValueOnElement } from './dom_utils';
+import { cloneHTMLElement, setValueOnElement } from './dom_utils';
 import morphdom from 'morphdom';
 import { normalizeAttributesForComparison } from './normalize_attributes_for_comparison';
 import Component from './Component';
@@ -17,16 +17,6 @@ export function executeMorphdom(
     const childComponentMap: Map<HTMLElement, Component> = new Map();
     childComponents.forEach((childComponent) => {
         childComponentMap.set(childComponent.element, childComponent);
-        if (!childComponent.id) {
-            throw new Error('Child is missing id.');
-        }
-        const childComponentToElement = findChildComponent(childComponent.id, rootToElement);
-        if (childComponentToElement && childComponentToElement.tagName !== childComponent.element.tagName) {
-            // we need to "correct" the tag name for the child to match the "from"
-            // so that we always get a "diff", not a remove/add
-            const newTag = cloneElementWithNewTagName(childComponentToElement, childComponent.element.tagName);
-            childComponentToElement.replaceWith(newTag);
-        }
     });
 
     morphdom(rootFromElement, rootToElement, {
@@ -55,7 +45,9 @@ export function executeMorphdom(
                 if (childComponentMap.has(fromEl)) {
                     const childComponent = childComponentMap.get(fromEl) as Component;
 
-                    return childComponent.updateFromNewElement(toEl);
+                    childComponent.updateFromNewElementFromParentRender(toEl);
+
+                    return false;
                 }
 
                 // if this field's value has been modified since this HTML was