bug #419 [LiveComponent] Avoiding model re-renders during an action (weaverryan)

This PR was merged into the 2.x branch.

Discussion
----------

[LiveComponent] Avoiding model re-renders during an action

| Q             | A
| ------------- | ---
| Bug fix?      | yes
| New feature?  | no
| Tickets       | Fixes issue reported on Slack by `@FrancoisPog`
| License       | MIT

This is so that we can guarantee that an action's response WILL always be used. Previously, an action's response would be ignored if a model-update re-render was triggered while the action's Ajax call was completing.

Cheers!

Commits
-------

73a2a3c Avoiding model re-renders during an action

Author: weaverryan

src/LiveComponent/CHANGELOG.md

@@ -1,5 +1,12 @@
 # CHANGELOG
 
+## 2.3.1
+
+-   [BEHAVIOR CHANGE] If an action Ajax call is still processing and a
+    model update occurs, the component will _no_ longer re-render. The
+    model will be updated internally, but not re-rendered (so, any
+    model updates would effectively have the `|norender` modifier). See #419.
+
 ## 2.3.0
 
 -   [BC BREAK] The `data-action="live#update"` attribute must now be

src/LiveComponent/assets/dist/live_controller.js

@@ -1084,8 +1084,8 @@ function getModelDirectiveFromInput(element, throwOnMissing = true) {
     }
     if (element.getAttribute('name')) {
         const formElement = element.closest('form');
-        if (formElement && formElement.dataset.model) {
-            const directives = parseDirectives(formElement.dataset.model);
+        if (formElement && ('model' in formElement.dataset)) {
+            const directives = parseDirectives(formElement.dataset.model || '*');
             const directive = directives[0];
             if (directive.args.length > 0 || directive.named.length > 0) {
                 throw new Error(`The data-model="${formElement.dataset.model}" format is invalid: it does not support passing arguments to the model.`);
@@ -1195,6 +1195,7 @@ class default_1 extends Controller {
         this.renderDebounceTimeout = null;
         this.actionDebounceTimeout = null;
         this.renderPromiseStack = new PromiseStack();
+        this.isActionProcessing = false;
         this.pollingIntervals = [];
         this.isWindowUnloaded = false;
         this.originalDataJSON = '{}';
@@ -1222,9 +1223,7 @@ class default_1 extends Controller {
         if (!(this.element instanceof HTMLElement)) {
             throw new Error('Invalid Element Type');
         }
-        if (this.element.dataset.poll !== undefined) {
-            this._initiatePolling(this.element.dataset.poll);
-        }
+        this._initiatePolling();
         window.addEventListener('beforeunload', this.markAsWindowUnloaded);
         this._startAttributesMutationObserver();
         this.element.addEventListener('live:update-model', this.handleUpdateModelEvent);
@@ -1234,9 +1233,7 @@ class default_1 extends Controller {
         this._dispatchEvent('live:connect', { controller: this });
     }
     disconnect() {
-        this.pollingIntervals.forEach((interval) => {
-            clearInterval(interval);
-        });
+        this._stopAllPolling();
         window.removeEventListener('beforeunload', this.markAsWindowUnloaded);
         this.element.removeEventListener('live:update-model', this.handleUpdateModelEvent);
         this.element.removeEventListener('input', this.handleInputEvent);
@@ -1387,7 +1384,7 @@ class default_1 extends Controller {
         }
         this.valueStore.set(modelName, value);
         this.unsyncedInputs.remove(modelName);
-        if (shouldRender) {
+        if (shouldRender && !this.isActionProcessing) {
             this._clearWaitingDebouncedRenders();
             let debounce = this.getDefaultDebounce();
             if (options.debounce !== undefined && options.debounce !== null) {
@@ -1412,6 +1409,7 @@ class default_1 extends Controller {
             'Accept': 'application/vnd.live-component+html',
         };
         if (action) {
+            this.isActionProcessing = true;
             url += `/${encodeURIComponent(action)}`;
             if (this.csrfValue) {
                 fetchOptions.headers['X-CSRF-TOKEN'] = this.csrfValue;
@@ -1437,6 +1435,9 @@ class default_1 extends Controller {
         const reRenderPromise = new ReRenderPromise(thisPromise, this.unsyncedInputs.clone());
         this.renderPromiseStack.addPromise(reRenderPromise);
         thisPromise.then((response) => {
+            if (action) {
+                this.isActionProcessing = false;
+            }
             if (this.renderDebounceTimeout) {
                 return;
             }
@@ -1665,7 +1666,12 @@ class default_1 extends Controller {
         }
         this._updateModelFromElement(target, 'change');
     }
-    _initiatePolling(rawPollConfig) {
+    _initiatePolling() {
+        this._stopAllPolling();
+        if (this.element.dataset.poll === undefined) {
+            return;
+        }
+        const rawPollConfig = this.element.dataset.poll;
         const directives = parseDirectives(rawPollConfig || '$render');
         directives.forEach((directive) => {
             let duration = 2000;
@@ -1790,9 +1796,12 @@ class default_1 extends Controller {
         const element = this.element;
         this.mutationObserver = new MutationObserver((mutations) => {
             mutations.forEach((mutation) => {
-                if (mutation.type === 'attributes' && !element.dataset.originalData) {
-                    this.originalDataJSON = this.valueStore.asJson();
-                    this._exposeOriginalData();
+                if (mutation.type === 'attributes') {
+                    if (!element.dataset.originalData) {
+                        this.originalDataJSON = this.valueStore.asJson();
+                        this._exposeOriginalData();
+                    }
+                    this._initiatePolling();
                 }
             });
         });
@@ -1803,6 +1812,11 @@ class default_1 extends Controller {
     getDefaultDebounce() {
         return this.hasDebounceValue ? this.debounceValue : DEFAULT_DEBOUNCE;
     }
+    _stopAllPolling() {
+        this.pollingIntervals.forEach((interval) => {
+            clearInterval(interval);
+        });
+    }
 }
 default_1.values = {
     url: String,

src/LiveComponent/assets/src/live_controller.ts

@@ -68,6 +68,8 @@ export default class extends Controller implements LiveController {
      */
     renderPromiseStack = new PromiseStack();
 
+    isActionProcessing = false;
+
     pollingIntervals: NodeJS.Timer[] = [];
 
     isWindowUnloaded = false;
@@ -393,7 +395,8 @@ export default class extends Controller implements LiveController {
         // any Ajax request that starts from this moment WILL include this
         this.unsyncedInputs.remove(modelName);
 
-        if (shouldRender) {
+        // skip rendering if there is an action Ajax call processing
+        if (shouldRender && !this.isActionProcessing) {
             // clear any pending renders
             this._clearWaitingDebouncedRenders();
 
@@ -425,6 +428,8 @@ export default class extends Controller implements LiveController {
         };
 
         if (action) {
+            this.isActionProcessing = true;
+
             url += `/${encodeURIComponent(action)}`;
 
             if (this.csrfValue) {
@@ -455,6 +460,10 @@ export default class extends Controller implements LiveController {
         const reRenderPromise = new ReRenderPromise(thisPromise, this.unsyncedInputs.clone());
         this.renderPromiseStack.addPromise(reRenderPromise);
         thisPromise.then((response) => {
+            if (action) {
+                this.isActionProcessing = false;
+            }
+
             // if another re-render is scheduled, do not "run it over"
             if (this.renderDebounceTimeout) {
                 return;

src/LiveComponent/assets/test/controller/action.test.ts

@@ -101,4 +101,52 @@ describe('LiveController Action Tests', () => {
 
         await waitFor(() => expect(test.element).toHaveTextContent('Food: pizza'));
    });
+
+    it('prevents re-render model updates while action Ajax is pending', async () => {
+        const test = await createTest({ comment: 'donut', isSaved: false }, (data: any) => `
+            <div ${initComponent(data)}>
+                <input data-model="comment" value="${data.comment}">
+
+                ${data.isSaved ? 'Comment Saved!' : ''}
+
+                <span>${data.comment}</span>
+
+                <button data-action="live#action" data-action-name="save">Save</button>
+                <button data-action="live#$render">Reload</button>
+            </div>
+        `);
+
+        // ONLY a post is sent, not a re-render GET
+        test.expectsAjaxCall('post')
+            .expectSentData(test.initialData)
+            .expectActionCalled('save')
+            .delayResponse(1000) // longer than debounce, so updating comment could potentially send a request
+            .serverWillChangeData((data: any) => {
+                // server marks component as "saved"
+                data.isSaved = true;
+            })
+            .init();
+
+        // save first, then type into the box
+        getByText(test.element, 'Save').click();
+        await userEvent.type(test.queryByDataModel('comment'), ' holes');
+
+        await waitFor(() => expect(test.element).toHaveTextContent('Comment Saved!'));
+        // render has not happened yet
+        expect(test.element).not.toHaveTextContent('donut holes');
+
+        // trigger a render, it should now reflect the changed value
+        test.expectsAjaxCall('get')
+            .expectSentData({comment: 'donut holes', isSaved: true})
+            .init();
+        getByText(test.element, 'Reload').click();
+        await waitFor(() => expect(test.element).toHaveTextContent('donut holes'));
+
+        // now check that model updating works again
+        test.expectsAjaxCall('get')
+            .expectSentData({comment: 'donut holes are delicious', isSaved: true})
+            .init();
+        await userEvent.type(test.queryByDataModel('comment'), ' are delicious');
+        await waitFor(() => expect(test.element).toHaveTextContent('donut holes are delicious'));
+    });
 });