feature #245 [Live] Always send HTML over the wire instead of JSON (weaverryan)

This PR was squashed before being merged into the 2.x branch.

Discussion
----------

[Live] Always send HTML over the wire instead of JSON

| Q             | A
| ------------- | ---
| Bug fix?      | yes (one part of it is)
| New feature?  | yes
| Tickets       | None
| License       | MIT

Hi!

Currently, the LiveComponent controller expects to get JSON back from the server, containing `{ data: ..., html: '...' }`. But, Stimulus is so cool, that this isn't necessary! The LiveComponent Ajax requests can simply return HTML in all cases. The rendered element will have an updated `data-live-data-value` attribute with the latest data. And so, the Stimulus component instantly reads that updated data and incorporates it: there is no need to send back the "component data" on a separate key in the JSON.

This makes the live component system more boring in a good way: it's just a way to return HTML (and then we have fancy JavaScript that adds all the interactivity). It's also a bit faster now, as previously we were dehydrating the component data twice on re-render.

I can't think of any downsides to this approach - everything seems to work just fine.

Cheers!

Commits
-------

f18d797 [Live] Always send HTML over the wire instead of JSON

Author: weaverryan

src/LiveComponent/CHANGELOG.md

@@ -1,5 +1,10 @@
 # CHANGELOG
 
+## 2.1.0
+
+-   The Live Component AJAX endpoints now return HTML in all situations
+    instead of JSON.
+
 ## 2.0.0
 
 -   Support for `stimulus` version 2 was removed and support for `@hotwired/stimulus`

src/LiveComponent/assets/src/live_controller.ts

@@ -6,12 +6,6 @@ import { buildFormData, buildSearchParams } from './http_data_helper';
 import { setDeepData, doesDeepPropertyExist, normalizeModelName } from './set_deep_data';
 import { haveRenderedValuesChanged } from './have_rendered_values_changed';
 
-interface LiveResponseData {
-    redirect_url?: string,
-    html?: string,
-    data?: any,
-}
-
 interface ElementLoadingDirectives {
     element: HTMLElement,
     directives: Directive[]
@@ -195,20 +189,8 @@ export default class extends Controller {
         this._makeRequest(null);
     }
 
-    _getValueFromElement(element: HTMLElement){
-        const value = element.dataset.value || element.value;
-
-        if (!value) {
-            const clonedElement = (element.cloneNode());
-            // helps typescript know this is an HTMLElement
-            if (!(clonedElement instanceof HTMLElement)) {
-                throw new Error('cloneNode() produced incorrect type');
-            }
-
-            throw new Error(`The update() method could not be called for "${clonedElement.outerHTML}": the element must either have a "data-value" or "value" attribute set.`);
-        }
-
-        return value;
+    _getValueFromElement(element: HTMLElement) {
+        return element.dataset.value || element.value;
     }
 
     _updateModelFromElement(element: HTMLElement, value: string, shouldRender: boolean) {
@@ -320,7 +302,7 @@ export default class extends Controller {
 
         const fetchOptions: RequestInit = {};
         fetchOptions.headers = {
-            'Accept': 'application/vnd.live-component+json',
+            'Accept': 'application/vnd.live-component+html',
         };
 
         if (action) {
@@ -351,8 +333,8 @@ export default class extends Controller {
 
             const isMostRecent = this.renderPromiseStack.removePromise(thisPromise);
             if (isMostRecent) {
-                response.json().then((data) => {
-                    this._processRerender(data)
+                response.text().then((html) => {
+                    this._processRerender(html, response);
                 });
             }
         })
@@ -363,24 +345,24 @@ export default class extends Controller {
      *
      * @private
      */
-    _processRerender(data: LiveResponseData) {
+    _processRerender(html: string, response: Response) {
         // check if the page is navigating away
         if (this.isWindowUnloaded) {
             return;
         }
 
-        if (data.redirect_url) {
+        if (response.headers.get('Location')) {
             // action returned a redirect
             if (typeof Turbo !== 'undefined') {
-                Turbo.visit(data.redirect_url);
+                Turbo.visit(response.headers.get('Location'));
             } else {
-                window.location.href = data.redirect_url;
+                window.location.href = response.headers.get('Location');
             }
 
             return;
         }
 
-        if (!this._dispatchEvent('live:render', data, true, true)) {
+        if (!this._dispatchEvent('live:render', html, true, true)) {
             // preventDefault() was called
             return;
         }
@@ -390,15 +372,8 @@ export default class extends Controller {
         // elements to appear different unnecessarily
         this._onLoadingFinish();
 
-        if (!data.html) {
-            throw new Error('Missing html key on response JSON');
-        }
-
         // merge/patch in the new HTML
-        this._executeMorphdom(data.html);
-
-        // "data" holds the new, updated data
-        this.dataValue = data.data;
+        this._executeMorphdom(html);
     }
 
     _clearWaitingDebouncedRenders() {
@@ -644,7 +619,7 @@ export default class extends Controller {
         this.pollingIntervals.push(timer);
     }
 
-    _dispatchEvent(name: string, payload: object | null = null, canBubble = true, cancelable = false) {
+    _dispatchEvent(name: string, payload: object | string | null = null, canBubble = true, cancelable = false) {
         return this.element.dispatchEvent(new CustomEvent(name, {
             bubbles: canBubble,
             cancelable,

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

@@ -48,10 +48,10 @@ describe('LiveController Action Tests', () => {
         const { element } = await startStimulus(template(data));
 
         // ONLY a post is sent, not a re-render GET
-        const postMock = fetchMock.postOnce('http://localhost/_components/my_component/save', {
-            html: template({ comments: 'hi weaver', isSaved: true }),
-            data: { comments: 'hi weaver', isSaved: true }
-        });
+        const postMock = fetchMock.postOnce(
+            'http://localhost/_components/my_component/save',
+            template({ comments: 'hi weaver', isSaved: true })
+        );
 
         await userEvent.type(getByLabelText(element, 'Comments:'), ' WEAVER');
 

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

@@ -133,6 +133,7 @@ describe('LiveController parent -> child component tests', () => {
         const inputElement = getByLabelText(element, 'Content:');
         await userEvent.clear(inputElement);
         await userEvent.type(inputElement, 'changed content');
+        // change the rows on the server
         mockRerender({'value': 'changed content'}, childTemplate, (data) => {
             data.rows = 5;
         });

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

@@ -47,10 +47,10 @@ describe('LiveController CSRF Tests', () => {
         const data = { comments: 'hi' };
         const { element } = await startStimulus(template(data));
 
-        const postMock = fetchMock.postOnce('http://localhost/_components/my_component/save', {
-            html: template({ comments: 'hi', isSaved: true }),
-            data: { comments: 'hi', isSaved: true }
-        });
+        const postMock = fetchMock.postOnce(
+            'http://localhost/_components/my_component/save',
+            template({ comments: 'hi', isSaved: true })
+        );
         getByText(element, 'Save').click();
 
         await waitFor(() => expect(element).toHaveTextContent('Comment Saved!'));

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

@@ -41,10 +41,7 @@ describe('LiveController data-model Tests', () => {
         const data = { name: 'Ryan' };
         const { element, controller } = await startStimulus(template(data));
 
-        fetchMock.getOnce('end:?name=Ryan+WEAVER', {
-            html: template({ name: 'Ryan Weaver' }),
-            data: { name: 'Ryan Weaver' }
-        });
+        fetchMock.getOnce('end:?name=Ryan+WEAVER', template({ name: 'Ryan Weaver' }));
 
         await userEvent.type(getByLabelText(element, 'Name:'), ' WEAVER', {
             // this tests the debounce: characters have a 10ms delay
@@ -66,10 +63,7 @@ describe('LiveController data-model Tests', () => {
         const data = { name: 'Ryan' };
         const { element, controller } = await startStimulus(template(data));
 
-        fetchMock.getOnce('end:?name=Jan', {
-            html: template({ name: 'Jan' }),
-            data: { name: 'Jan' }
-        });
+        fetchMock.getOnce('end:?name=Jan', template({ name: 'Jan' }));
 
         userEvent.click(getByText(element, 'Change name to Jan'));
 
@@ -96,12 +90,11 @@ describe('LiveController data-model Tests', () => {
             ['guy', 150]
         ];
         requests.forEach(([string, delay]) => {
-            fetchMock.getOnce(`end:my_component?name=Ryan${string}`, {
-                // the _ at the end helps us look that the input has changed
-                // as a result of a re-render (not just from typing in the input)
-                html: template({ name: `Ryan${string}_` }),
-                data: { name: `Ryan${string}_` }
-            }, { delay });
+            fetchMock.getOnce(
+                `end:my_component?name=Ryan${string}`,
+                template({ name: `Ryan${string}_` }),
+                { delay }
+            );
         });
 
         await userEvent.type(getByLabelText(element, 'Name:'), 'guy', {
@@ -275,4 +268,22 @@ describe('LiveController data-model Tests', () => {
         // assert all calls were done the correct number of times
         fetchMock.done();
     });
+
+    it('data changed on server should be noticed by controller', async () => {
+        const data = { name: 'Ryan' };
+        const { element, controller } = await startStimulus(template(data));
+
+        mockRerender({name: 'Ryan WEAVER'}, template, (data) => {
+            // sneaky server changes the data!
+            data.name = 'Kevin Bond';
+        });
+
+        const inputElement = getByLabelText(element, 'Name:');
+        await userEvent.type(inputElement, ' WEAVER');
+
+        await waitFor(() => expect(inputElement).toHaveValue('Kevin Bond'));
+        expect(controller.dataValue).toEqual({name: 'Kevin Bond'});
+
+        fetchMock.done();
+    });
 });

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

@@ -61,12 +61,11 @@ describe('LiveController rendering Tests', () => {
         const data = { name: 'Ryan' };
         const { element } = await startStimulus(template(data));
 
-        fetchMock.get('http://localhost/_components/my_component?name=Ryan', {
-            html: template({ name: 'Kevin' }),
-            data: { name: 'Kevin' }
-        }, {
-            delay: 100
-        });
+        fetchMock.get(
+            'http://localhost/_components/my_component?name=Ryan',
+            template({ name: 'Kevin' }),
+            { delay: 100 }
+        );
         getByText(element, 'Reload').click();
         userEvent.type(getByLabelText(element, 'Comments:'), '!!');
 
@@ -84,12 +83,11 @@ describe('LiveController rendering Tests', () => {
             template(data, true)
         );
 
-        fetchMock.get('http://localhost/_components/my_component?name=Ryan', {
-            html: template({ name: 'Kevin' }, true),
-            data: { name: 'Kevin' }
-        }, {
-            delay: 100
-        });
+        fetchMock.get(
+            'http://localhost/_components/my_component?name=Ryan',
+            template({ name: 'Kevin' }, true),
+            { delay: 100 }
+        );
         getByText(element, 'Reload').click();
         userEvent.type(getByLabelText(element, 'Comments:'), '!!');
 
@@ -102,10 +100,7 @@ describe('LiveController rendering Tests', () => {
         const data = { name: 'Ryan' };
         const { element } = await startStimulus(template(data));
 
-        fetchMock.get('end:?name=Ryan', {
-            html: '<div>aloha!</div>',
-            data: { name: 'Kevin' }
-        }, {
+        fetchMock.get('end:?name=Ryan', '<div>aloha!</div>', {
             delay: 100
         });
 

src/LiveComponent/assets/test/tools.ts

@@ -74,10 +74,7 @@ const mockRerender = (sentData, renderCallback, changeDataCallback = null) => {
         changeDataCallback(sentData);
     }
 
-    fetchMock.mock(url, {
-        html: renderCallback(sentData),
-        data: sentData
-    });
+    fetchMock.mock(url, renderCallback(sentData));
 }
 
 export { startStimulus, getControllerElement, initLiveComponent, mockRerender };

src/LiveComponent/src/EventListener/LiveComponentSubscriber.php

@@ -13,7 +13,6 @@
 
 use Psr\Container\ContainerInterface;
 use Symfony\Component\EventDispatcher\EventSubscriberInterface;
-use Symfony\Component\HttpFoundation\JsonResponse;
 use Symfony\Component\HttpFoundation\Request;
 use Symfony\Component\HttpFoundation\Response;
 use Symfony\Component\HttpKernel\Event\ControllerEvent;
@@ -41,8 +40,7 @@
  */
 class LiveComponentSubscriber implements EventSubscriberInterface, ServiceSubscriberInterface
 {
-    private const JSON_FORMAT = 'live-component-json';
-    private const JSON_CONTENT_TYPE = 'application/vnd.live-component+json';
+    private const HTML_CONTENT_TYPE = 'application/vnd.live-component+html';
 
     private ContainerInterface $container;
 
@@ -69,8 +67,6 @@ public function onKernelRequest(RequestEvent $event): void
             return;
         }
 
-        $request->setFormat(self::JSON_FORMAT, self::JSON_CONTENT_TYPE);
-
         // the default "action" is get, which does nothing
         $action = $request->get('action', 'get');
         $componentName = (string) $request->get('component');
@@ -192,16 +188,17 @@ public function onKernelResponse(ResponseEvent $event): void
             return;
         }
 
-        if (!$this->isLiveComponentJsonRequest($request)) {
+        if (!\in_array(self::HTML_CONTENT_TYPE, $request->getAcceptableContentTypes(), true)) {
             return;
         }
 
         if (!$response->isRedirection()) {
             return;
         }
 
-        $event->setResponse(new JsonResponse([
-            'redirect_url' => $response->headers->get('Location'),
+        $event->setResponse(new Response(null, 204, [
+            'Location' => $response->headers->get('Location'),
+            'Content-Type' => self::HTML_CONTENT_TYPE,
         ]));
     }
 
@@ -227,27 +224,11 @@ private function createResponse(object $component, Request $request): Response
             $request->attributes->get('_component_template')
         );
 
-        if ($this->isLiveComponentJsonRequest($request)) {
-            return new JsonResponse(
-                [
-                    'html' => $html,
-                    'data' => $this->container->get(LiveComponentHydrator::class)->dehydrate($component),
-                ],
-                200,
-                ['Content-Type' => self::JSON_CONTENT_TYPE]
-            );
-        }
-
         return new Response($html);
     }
 
     private function isLiveComponentRequest(Request $request): bool
     {
         return 'live_component' === $request->attributes->get('_route');
     }
-
-    private function isLiveComponentJsonRequest(Request $request): bool
-    {
-        return \in_array($request->getPreferredFormat(), [self::JSON_FORMAT, 'json'], true);
-    }
 }