Fixing bug where polling doesn't stop after data-poll is removed

Author: weaverryan

src/LiveComponent/assets/src/live_controller.ts

@@ -108,9 +108,7 @@ export default class extends Controller implements LiveController {
             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();
@@ -123,9 +121,7 @@ export default class extends Controller implements LiveController {
     }
 
     disconnect() {
-        this.pollingIntervals.forEach((interval) => {
-            clearInterval(interval);
-        });
+        this._stopAllPolling();
 
         window.removeEventListener('beforeunload', this.markAsWindowUnloaded);
         this.element.removeEventListener('live:update-model', this.handleUpdateModelEvent);
@@ -794,7 +790,14 @@ export default class extends Controller implements LiveController {
         this._updateModelFromElement(target, 'change')
     }
 
-    _initiatePolling(rawPollConfig: string) {
+    _initiatePolling() {
+        this._stopAllPolling();
+
+        if ((this.element as HTMLElement).dataset.poll === undefined) {
+            return;
+        }
+
+        const rawPollConfig = (this.element as HTMLElement).dataset.poll;
         const directives = parseDirectives(rawPollConfig || '$render');
 
         directives.forEach((directive) => {
@@ -959,7 +962,10 @@ export default class extends Controller implements LiveController {
     }
 
     /**
-     * Re-establishes the data-original-data attribute if missing.
+     * Helps "re-normalize" certain root element attributes after a re-render.
+     *
+     *      1) Re-establishes the data-original-data attribute if missing.
+     *      2) Stops or re-initializes data-poll
      *
      * This happens if a parent component re-renders a child component
      * and morphdom *updates* child. This commonly happens if a parent
@@ -979,9 +985,13 @@ export default class extends Controller implements LiveController {
 
         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();
                 }
             });
         });
@@ -994,6 +1004,12 @@ export default class extends Controller implements LiveController {
     private getDefaultDebounce(): number {
         return this.hasDebounceValue ? this.debounceValue : DEFAULT_DEBOUNCE;
     }
+
+    private _stopAllPolling() {
+        this.pollingIntervals.forEach((interval) => {
+            clearInterval(interval);
+        });
+    }
 }
 
 /**

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

@@ -10,9 +10,7 @@
 'use strict';
 
 import { shutdownTest, createTest, initComponent } from '../tools';
-import { createEvent, fireEvent, getByText, waitFor } from '@testing-library/dom';
-import userEvent from '@testing-library/user-event';
-import fetchMock from 'fetch-mock-jest';
+import { waitFor } from '@testing-library/dom';
 
 describe('LiveController polling Tests', () => {
     afterEach(() => {
@@ -112,4 +110,79 @@ describe('LiveController polling Tests', () => {
             timeout: 600
         });
     });
+
+    // check polling stops after disconnect
+
+    it('polling should stop if data-poll is removed', async () => {
+        const test = await createTest({ keepPolling: true, renderCount: 0 }, (data: any) => `
+            <div ${initComponent(data)} ${data.keepPolling ? 'data-poll="delay(500)|$render"' : ''}>
+                <span>Render count: ${data.renderCount}</span>
+            </div>
+        `);
+
+        // poll 1
+        test.expectsAjaxCall('get')
+            .expectSentData(test.initialData)
+            .serverWillChangeData((data: any) => {
+                data.renderCount = 1;
+            })
+            .init();
+        // poll 2
+        test.expectsAjaxCall('get')
+            .expectSentData({keepPolling: true, renderCount: 1})
+            .serverWillChangeData((data: any) => {
+                data.renderCount = 2;
+                data.keepPolling = false;
+            })
+            .init();
+
+        // only wait for about 500ms this time
+        await waitFor(() => expect(test.element).toHaveTextContent('Render count: 1'), {
+            timeout: 600
+        });
+        await waitFor(() => expect(test.element).toHaveTextContent('Render count: 2'), {
+            timeout: 600
+        });
+        // wait 1 more second... no more Ajax calls should be made
+        const timeoutPromise = new Promise((resolve) => {
+            setTimeout(() => {
+                resolve(true);
+            }, 1000);
+        });
+        await waitFor(() => timeoutPromise, {
+            timeout: 1500
+        });
+    });
+
+    it('stops polling after it disconnects', async () => {
+       const test = await createTest({ renderCount: 0 }, (data: any) => `
+           <div ${initComponent(data)} data-poll="delay(500)|$render">
+               <span>Render count: ${data.renderCount}</span>
+           </div>
+       `);
+
+       // poll 1
+       test.expectsAjaxCall('get')
+           .expectSentData(test.initialData)
+           .serverWillChangeData((data: any) => {
+               data.renderCount = 1;
+           })
+           .init();
+
+       // only wait for about 500ms this time
+       await waitFor(() => expect(test.element).toHaveTextContent('Render count: 1'), {
+           timeout: 600
+       });
+       // "remove" our controller from the page
+       document.body.innerHTML = '<div>something else</div>';
+        // wait 1 more second... no more Ajax calls should be made
+        const timeoutPromise = new Promise((resolve) => {
+            setTimeout(() => {
+                resolve(true);
+            }, 1000);
+        });
+        await waitFor(() => timeoutPromise, {
+            timeout: 1500
+        });
+   });
 });