Adding logic to "smartly" re-render child components

weaverryan · weaverryan · commit 84a3bf0c40e7 · 2021-07-06T11:29:14.000-04:00
diff --git a/src/LiveComponent/README.md b/src/LiveComponent/README.md
@@ -1151,9 +1151,33 @@ Here are a few helpful things to know:
 
 ### Each component re-renders independent of one another
 
-If a parent component re-renders, the child component will *not* be updated,
-even though it lives inside the parent. Each component is its own,
-isolated universe.
+If a parent component re-renders, the child component will *not* (most
+of the time) be updated,  even though it lives inside the parent. Each
+component is its own, isolated universe.
+
+But... this is not always what you want. For example, suppose a
+parent component holds a form and a child component holds one field.
+When you click a "Save" button on the parent component, it validates
+the form and re-renders with errors - including a new `error` value
+that it passes into the child:
+
+```twig
+{# templates/components/post_form.html.twig #}
+
+{{ component('textarea_field', {
+    value: this.content,
+    error: this.getError('content')
+}) }}
+```
+
+In this situation, when the parent component re-renders after clicking
+"Save", you *do* want the updated child  component (with the validation
+error) to be rendered. And, this *will* happen because the system detects
+that the parent component has *changed* how it's rendering the child.
+
+This may not always be perfect, and if your child component has its own
+`LiveProp` that has changed since it was first rendered, that value will
+be lost when the parent component causes the child to re-render.
 
 ### Actions, methods and model updates in a child do not affect the parent
 
diff --git a/src/LiveComponent/assets/src/have_rendered_values_changed.js b/src/LiveComponent/assets/src/have_rendered_values_changed.js
@@ -0,0 +1,70 @@
+export function haveRenderedValuesChanged(originalDataJson, currentDataJson, newDataJson) {
+    /*
+     * Right now, if the "data" on the new value is different than
+     * the "original data" on the child element, then we force re-render
+     * the child component. There may be some other cases that we
+     * add later if they come up. Either way, this is probably the
+     * behavior we want most of the time, but it's not perfect. For
+     * example, if the child component has some a writable prop that
+     * has changed since the initial render, re-rendering the child
+     * component from the parent component will "eliminate" that
+     * change.
+     */
+
+    // if the original data matches the new data, then the parent
+    // hasn't changed how they render the child.
+    if (originalDataJson === newDataJson) {
+        return false;
+    }
+
+    // The child component IS now being rendered in a "new way".
+    // This means that at least one of the "data" pieces used
+    // to render the child component has changed.
+    // However, the piece of data that changed might simply
+    // match the "current data" of that child component. In that case,
+    // there is no point to re-rendering.
+    // And, to be safe (in case the child has some "private LiveProp"
+    // that has been modified), we want to avoid rendering.
+
+
+    // if the current data exactly matches the new data, then definitely
+    // do not re-render.
+    if (currentDataJson === newDataJson) {
+        return false;
+    }
+
+    // here, we will compare the original data for the child component
+    // with the new data. What we're looking for are they keys that
+    // have changed between the original "child rendering" and the
+    // new "child rendering".
+    const originalData = JSON.parse(originalDataJson);
+    const newData = JSON.parse(newDataJson);
+    const changedKeys = Object.keys(newData);
+    Object.entries(originalData).forEach(([key, value]) => {
+        // if any key in the new data is different than a key in the
+        // current data, then we *should* re-render. But if all the
+        // keys in the new data equal
+        if (value === newData[key]) {
+            // value is equal, remove from changedKeys
+            changedKeys.splice(changedKeys.indexOf(key), 1);
+        }
+    });
+
+    // now that we know which keys have changed between originally
+    // rendering the child component and this latest render, we
+    // can check to see if the the child component *already* has
+    // the latest value for those keys.
+
+    const currentData = JSON.parse(currentDataJson)
+    let keyHasChanged = false;
+    changedKeys.forEach((key) => {
+        // if any key in the new data is different than a key in the
+        // current data, then we *should* re-render. But if all the
+        // keys in the new data equal
+        if (currentData[key] !== newData[key]) {
+            keyHasChanged = true;
+        }
+    });
+
+    return keyHasChanged;
+}
diff --git a/src/LiveComponent/assets/src/live_controller.js b/src/LiveComponent/assets/src/live_controller.js
@@ -5,6 +5,7 @@ import { combineSpacedArray } from './string_utils';
 import { buildFormData, buildSearchParams } from './http_data_helper';
 import { setDeepData, doesDeepPropertyExist, normalizeModelName } from './set_deep_data';
 import './polyfills';
+import { haveRenderedValuesChanged } from './have_rendered_values_changed';
 
 const DEFAULT_DEBOUNCE = '150';
 
@@ -44,8 +45,12 @@ export default class extends Controller {
 
     isWindowUnloaded = false;
 
+    originalDataJSON;
+
     initialize() {
         this.markAsWindowUnloaded = this.markAsWindowUnloaded.bind(this);
+        this.originalDataJSON = JSON.stringify(this.dataValue);
+        this._exposeOriginalData();
     }
 
     connect() {
@@ -530,13 +535,16 @@ export default class extends Controller {
                 if (fromEl.hasAttribute('data-controller')
                     && fromEl.getAttribute('data-controller').split(' ').indexOf('live') !== -1
                     && fromEl !== this.element
+                    && !this._shouldChildLiveElementUpdate(fromEl, toEl)
                 ) {
                     return false;
                 }
 
                 return true;
             }
         });
+        // restore the data-original-data attribute
+        this._exposeOriginalData();
     }
 
     markAsWindowUnloaded = () => {
@@ -616,6 +624,31 @@ export default class extends Controller {
             }
         );
     }
+
+    /**
+     * Determines of a child live element should be re-rendered.
+     *
+     * This is called when this element re-renders and detects that
+     * a child element is inside. Normally, in that case, we do not
+     * re-render the child element. However, if we detect that the
+     * "data" on the child element has changed from its initial data,
+     * then this will trigger a re-render.
+     *
+     * @param {Element} fromEl
+     * @param {Element} toEl
+     * @return {boolean}
+     */
+    _shouldChildLiveElementUpdate(fromEl, toEl) {
+        return haveRenderedValuesChanged(
+            fromEl.dataset.originalData,
+            fromEl.dataset.liveDataValue,
+            toEl.dataset.liveDataValue
+        );
+    }
+
+    _exposeOriginalData() {
+        this.element.dataset.originalData = this.originalDataJSON;
+    }
 }
 
 /**
diff --git a/src/LiveComponent/assets/test/controller/child.test.js b/src/LiveComponent/assets/test/controller/child.test.js
@@ -10,28 +10,33 @@
 'use strict';
 
 import { clearDOM } from '@symfony/stimulus-testing';
-import { startStimulus } from '../tools';
-import { createEvent, fireEvent, getByLabelText, getByText, waitFor } from '@testing-library/dom';
+import { initLiveComponent, mockRerender, startStimulus } from '../tools';
+import { getByLabelText, getByText, waitFor } from '@testing-library/dom';
 import userEvent from '@testing-library/user-event';
 import fetchMock from 'fetch-mock-jest';
 
 describe('LiveController parent -> child component tests', () => {
     const parentTemplate = (data) => {
+        const errors = data.errors || { post: {} };
+
         return `
             <div
-                data-controller="live"
-                data-live-url-value="http://localhost/_components/parent"
+                ${initLiveComponent('/_components/parent', data)}
             >
                 <span>Title: ${data.post.title}</span>
                 <span>Description in Parent: ${data.post.content}</span>
 
-                <input
-                    type="text"
-                    name="post[title]"
-                    value="${data.post.title}"
-                >
+                <label>
+                    Title:
+                    <input
+                        type="text"
+                        name="post[title]"
+                        value="${data.post.title}"
+                        data-action="live#update"
+                    >
+                </label>
 
-                ${childTemplate({ value: data.post.content })}
+                ${childTemplate({ value: data.post.content, error: errors.post.content })}
 
                 <button
                     data-action="live#$render"
@@ -42,82 +47,66 @@ describe('LiveController parent -> child component tests', () => {
 
     const childTemplate = (data) => `
         <div
-            data-controller="live"
-            data-live-url-value="http://localhost/_components/child"
+            ${initLiveComponent('/_components/child', data)}
         >
-            <!-- form field not mapped with data-model -->
             <label>
                 Content:
-                <input
-                    value="${data.value}"
+                <textarea
                     data-model="value"
                     name="post[content]"
                     data-action="live#update"
-                >
+                    rows="${data.rows ? data.rows : '3'}"
+                >${data.value}</textarea>
             </label>
 
             <div>Value in child: ${data.value}</div>
+            <div>Error in child: ${data.error ? data.error : 'none'}</div>
+            {# Rows represents a writable prop that's private to the child component #}
+            <div>Rows in child: ${data.rows ? data.rows : 'not set'}</div>
 
             <button
                 data-action="live#$render"
             >Child Re-render</button>
         </div>
     `;
 
-    const startStimulusForParentChild = async (html, data, childData) => {
-        const testData = await startStimulus(
-            parentTemplate(data),
-            data
-        );
-
-        // setup the values on the child element
-        testData.element.querySelector('[data-controller="live"]').dataset.liveDataValue = JSON.stringify(childData);
-
-        return testData;
-    }
-
     afterEach(() => {
         clearDOM();
         fetchMock.reset();
     });
 
     it('renders parent component without affecting child component', async () => {
-        const data = { post: { title: 'Parent component', content: 'i love components' } };
-        const { element } = await startStimulusForParentChild(
-            parentTemplate(data),
-            data,
-            { value: data.post.content }
-        );
-
-        // on child re-render, use a new content
-        fetchMock.get('http://localhost/_components/child?value=i+love+components', {
-            html: childTemplate({ value: 'i love popcorn' }),
-            data: { value: 'i love popcorn' }
+        const data = { post: { title: 'Parent component', content: 'i love' } };
+        const { element } = await startStimulus(parentTemplate(data));
+
+        // on child re-render, expect the new value, change rows on the server
+        mockRerender({ value: 'i love popcorn' }, childTemplate, (data) => {
+            // change the "rows" data on the "server"
+            data.rows = 5;
         });
+        await userEvent.type(getByLabelText(element, 'Content:'), ' popcorn');
 
-        // reload the child template
-        getByText(element, 'Child Re-render').click();
         await waitFor(() => expect(element).toHaveTextContent('Value in child: i love popcorn'));
-
-        // on parent re-render, render the child template differently
-        fetchMock.get('begin:http://localhost/_components/parent', {
-            html: parentTemplate({ post: { title: 'Changed title', content: 'changed content' } }),
-            data: { post: { title: 'Changed title', content: 'changed content' } }
-        });
-        getByText(element, 'Parent Re-render').click();
-        await waitFor(() => expect(element).toHaveTextContent('Title: Changed title'));
+        expect(element).toHaveTextContent('Rows in child: 5');
+
+        // when the parent re-renders, expect the changed title AND content (from child)
+        // but, importantly, the only "changed" data that will be passed into
+        // the child component will be "content", which will match what the
+        // child already has. This will NOT trigger a re-render.
+        mockRerender(
+            { post: { title: 'Parent component changed', content: 'i love popcorn' } },
+            parentTemplate
+        )
+        await userEvent.type(getByLabelText(element, 'Title:'), ' changed');
+        await waitFor(() => expect(element).toHaveTextContent('Title: Parent component changed'));
 
         // the child component should *not* have updated
-        expect(element).toHaveTextContent('Value in child: i love popcorn');
+        expect(element).toHaveTextContent('Rows in child: 5');
     });
 
     it('updates child model and parent model in a deferred way', async () => {
         const data = { post: { title: 'Parent component', content: 'i love' } };
-        const { element, controller } = await startStimulusForParentChild(
-            parentTemplate(data),
-            data,
-            { value: data.post.content }
-        );
+        const { element, controller } = await startStimulus(parentTemplate(data));
 
         // verify the child request contains the correct description & re-render
         fetchMock.get('http://localhost/_components/child?value=i+love+turtles', {
@@ -139,6 +128,43 @@ describe('LiveController parent -> child component tests', () => {
         expect(controller.dataValue.post.content).toEqual('i love turtles');
     });
 
+    it('updates re-renders a child component if data has changed from initial', async () => {
+        const data = { post: { title: 'Parent component', content: 'initial content' } };
+        const { element } = await startStimulus(parentTemplate(data));
+
+        // allow the child to re-render, but change the "rows" value
+        const inputElement = getByLabelText(element, 'Content:');
+        await userEvent.clear(inputElement);
+        await userEvent.type(inputElement, 'changed content');
+        fetchMock.get('http://localhost/_components/child?value=changed+content', {
+            html: childTemplate({ value: 'changed content', rows: 5 }),
+            data: { value: 'changed content', rows: 5 }
+        });
+
+        // reload, which will give us rows=5
+        getByText(element, 'Child Re-render').click();
+        await waitFor(() => expect(element).toHaveTextContent('Rows in child: 5'));
+
+        // simulate an action in the parent component where "errors" changes
+        const newData = {...data};
+        newData.post.title = 'Changed title';
+        newData.post.content = 'changed content';
+        newData.errors = { post: { content: 'the content is not interesting enough' }};
+        fetchMock.get('http://localhost/_components/parent?post%5Btitle%5D=Parent+component&post%5Bcontent%5D=changed+content', {
+            html: parentTemplate(newData),
+            data: newData
+        });
+
+        getByText(element, 'Parent Re-render').click();
+        await waitFor(() => expect(element).toHaveTextContent('Title: Changed title'));
+        // the child, of course, still has the "changed content" value
+        expect(element).toHaveTextContent('Value in child: changed content');
+        // but because some child data *changed* from its original value, the child DOES re-render
+        expect(element).toHaveTextContent('Error in child: the content is not interesting enough');
+        // however, this means that the updated "rows" data on the child is lost
+        expect(element).toHaveTextContent('Rows in child: not set');
+    });
+
     // TODO - what if a child component re-renders and comes down with
     // a changed set of data? Should that update the parent's data?
 });
diff --git a/src/LiveComponent/assets/test/have_rendered_values_changed.test.js b/src/LiveComponent/assets/test/have_rendered_values_changed.test.js
diff --git a/src/LiveComponent/assets/test/tools.js b/src/LiveComponent/assets/test/tools.js
