From d82ee7a2bffe6efd01d778be692bc097d6416c75 Mon Sep 17 00:00:00 2001 From: Ryan Weaver Date: Wed, 25 Oct 2023 16:24:55 -0400 Subject: [PATCH] [LiveComponent] Trigger model:set if a prop changes on the server --- .../assets/src/Component/ValueStore.ts | 32 ++- .../assets/src/Component/index.ts | 6 +- .../assets/test/Component/index.test.ts | 192 ++++++++++++++---- .../assets/test/ValueStore.test.ts | 37 ++++ src/LiveComponent/assets/test/tools.ts | 2 +- src/LiveComponent/doc/index.rst | 2 + .../src/LiveComponentHydrator.php | 10 +- 7 files changed, 224 insertions(+), 57 deletions(-) diff --git a/src/LiveComponent/assets/src/Component/ValueStore.ts b/src/LiveComponent/assets/src/Component/ValueStore.ts index ae07374372d..cdad9b724d1 100644 --- a/src/LiveComponent/assets/src/Component/ValueStore.ts +++ b/src/LiveComponent/assets/src/Component/ValueStore.ts @@ -4,8 +4,6 @@ import { normalizeModelName } from '../string_utils'; export default class { /** * Original, read-only props that represent the original component state. - * - * @private */ private props: any = {}; @@ -104,10 +102,14 @@ export default class { /** * Called when an update request finishes successfully. */ - reinitializeAllProps(props: any): void { + reinitializeAllProps(props: any): string[] { + const changedProps = this.deepDiff(props); + this.props = props; this.updatedPropsFromParent = {}; this.pendingProps = {}; + + return changedProps; } /** @@ -138,8 +140,6 @@ export default class { for (const [key, value] of Object.entries(props)) { const currentValue = this.get(key); - // if the readonly identifier is different, then overwrite the - // prop entirely if (currentValue !== value) { changed = true; } @@ -151,4 +151,26 @@ export default class { return changed; } + + private deepDiff(newObj: any, prefix = '', changedProps: string[] = []): string[] { + for (const [key, value] of Object.entries(newObj)) { + const currentPath = prefix ? `${prefix}.${key}` : key; + const currentValue = this.get(currentPath); + + if (currentValue !== value) { + if (typeof currentValue !== 'object' || typeof value !== 'object') { + // if a prop is dirty, the prop hasn't changed, because the dirty value will take precedence + if (this.dirtyProps[currentPath] !== undefined) { + continue; + } + + changedProps.push(currentPath); + } else { + this.deepDiff(value, currentPath, changedProps); + } + } + } + + return changedProps; + } } diff --git a/src/LiveComponent/assets/src/Component/index.ts b/src/LiveComponent/assets/src/Component/index.ts index f3c22905f8a..6062cf10081 100644 --- a/src/LiveComponent/assets/src/Component/index.ts +++ b/src/LiveComponent/assets/src/Component/index.ts @@ -472,7 +472,7 @@ export default class Component { } const newProps = this.elementDriver.getComponentProps(newElement); - this.valueStore.reinitializeAllProps(newProps); + const changedProps = this.valueStore.reinitializeAllProps(newProps); const eventsToEmit = this.elementDriver.getEventsToEmit(newElement); const browserEventsToDispatch = this.elementDriver.getBrowserEventsToDispatch(newElement); @@ -497,6 +497,10 @@ export default class Component { this.valueStore.set(modelName, modifiedModelValues[modelName]); }); + changedProps.forEach((modelName) => { + this.hooks.triggerHook('model:set', modelName, this.valueStore.get(modelName), this); + }); + eventsToEmit.forEach(({ event, data, target, componentName }) => { if (target === 'up') { this.emitUp(event, data, componentName); diff --git a/src/LiveComponent/assets/test/Component/index.test.ts b/src/LiveComponent/assets/test/Component/index.test.ts index 8a558dbc071..a1d8887b61e 100644 --- a/src/LiveComponent/assets/test/Component/index.test.ts +++ b/src/LiveComponent/assets/test/Component/index.test.ts @@ -5,74 +5,176 @@ import BackendRequest from '../../src/Backend/BackendRequest'; import { Response } from 'node-fetch'; import { waitFor } from '@testing-library/dom'; import BackendResponse from '../../src/Backend/BackendResponse'; +import { dataToJsonAttribute } from '../tools'; + +class ComponentTest { + component: Component; + backend: BackendInterface; + + calledActions: BackendAction[] = []; + mockedResponses: string[] = []; + currentMockedResponse = 0; + + constructor(props: any) { + // eslint-disable-next-line @typescript-eslint/no-this-alias + const componentTest = this; + this.backend = { + makeRequest(props: any, actions: BackendAction[]): BackendRequest { + componentTest.calledActions = actions; + + if (!componentTest.mockedResponses[componentTest.currentMockedResponse]) { + throw new Error(`No mocked response for request #${componentTest.currentMockedResponse}`); + } + const html = componentTest.mockedResponses[componentTest.currentMockedResponse]; + componentTest.currentMockedResponse++; + + return new BackendRequest( + // @ts-ignore Response doesn't quite match the underlying interface + new Promise((resolve) => resolve(new Response(html, { + headers: { + 'Content-Type': 'application/vnd.live-component+html', + } + }))), + [], + [], + ); + }, + }; + + this.component = new Component( + document.createElement('div'), + 'test-component', + props, + [], + () => [], + null, + null, + this.backend, + new StandardElementDriver(), + ); + } -interface MockBackend extends BackendInterface { - actions: BackendAction[], -} - -const makeTestComponent = (): { component: Component, backend: MockBackend } => { - const backend: MockBackend = { - actions: [], - makeRequest(data: any, actions: BackendAction[]): BackendRequest { - this.actions = actions; - - return new BackendRequest( - // @ts-ignore Response doesn't quite match the underlying interface - new Promise((resolve) => resolve(new Response('
'))), - [], - [] - ) - } + addMockResponse(html: string): void { + this.mockedResponses.push(html); } - const component = new Component( - document.createElement('div'), - 'test-component', - { firstName: '' }, - [], - () => [], - null, - null, - backend, - new StandardElementDriver() - ); - - return { - component, - backend + getPendingResponseCount(): number { + return this.mockedResponses.length - this.currentMockedResponse; } } +let currentTest: ComponentTest|null = null; +const createTest = (props: any): ComponentTest => { + return currentTest = new ComponentTest(props); +}; + describe('Component class', () => { + afterEach(() => { + if (currentTest) { + if (currentTest.getPendingResponseCount() > 0) { + throw new Error(`Test finished with ${currentTest.getPendingResponseCount()} pending responses`); + } + } + currentTest = null; + }); + describe('set() method', () => { it('returns a Promise that eventually resolves', async () => { - const { component } = makeTestComponent(); + const test = createTest({ + firstName: '', + }); + test.addMockResponse('
'); let backendResponse: BackendResponse|null = null; // set model but no re-render - const promise = component.set('firstName', 'Ryan', false); + const promise = test.component.set('firstName', 'Ryan', false); // when this promise IS finally resolved, set the flag to true promise.then((response) => backendResponse = response); - // it should not have happened yet + // even if we wait for a potential response to resolve, it won't resolve the promise yet + await (new Promise(resolve => setTimeout(resolve, 10))); expect(backendResponse).toBeNull(); // set model WITH re-render - component.set('firstName', 'Kevin', true); - // it's still not *instantly* resolve - it'll + test.component.set('firstName', 'Kevin', true); + // it's still not *instantly* resolved expect(backendResponse).toBeNull(); await waitFor(() => expect(backendResponse).not.toBeNull()); // @ts-ignore expect(await backendResponse?.getBody()).toEqual('
'); }); + + it('triggers the model:set hook', async () => { + const test = createTest({ + firstName: '', + }); + + let hookCalled = false; + let actualModel: string|null = null; + let actualValue: string|null = null; + let actualComponent: Component|null = null; + test.component.on('model:set', (model, value, theComponent) => { + hookCalled = true; + actualModel = model; + actualValue = value; + actualComponent = theComponent; + }); + test.component.set('firstName', 'Ryan', false); + expect(hookCalled).toBe(true); + expect(actualModel).toBe('firstName'); + expect(actualValue).toBe('Ryan'); + expect(actualComponent).toBe(test.component); + }); + }); + + describe('render() method', () => { + it('triggers model:set hook if a model changes on the server', async () => { + const test = createTest({ + firstName: '', + product: { + id: 5, + name: 'cool stuff', + }, + lastName: '', + }); + + const newProps = { + firstName: 'Ryan', + lastName: 'Bond', + product: { + id: 5, + name: 'purple stuff', + }, + }; + test.addMockResponse(`
`); + + const promise = test.component.render(); + + // During the request, change lastName to make it a "dirty change" + // The new value from the server is effectively ignored, and so no + // model:set hook should be triggered + test.component.set('lastName', 'dirty change', false); + + const hookModels: string[] = []; + test.component.on('model:set', (model) => { + hookModels.push(model); + }); + + await promise; + + expect(hookModels).toEqual(['firstName', 'product.name']); + }); }); describe('Proxy wrapper', () => { - const makeDummyComponent = (): { proxy: Component, backend: MockBackend } => { - const { backend, component} = makeTestComponent(); + const makeDummyComponent = (): { proxy: Component, test: ComponentTest } => { + const test = createTest({ + firstName: '', + }); + return { - proxy: proxifyComponent(component), - backend + proxy: proxifyComponent(test.component), + test, } } @@ -108,16 +210,16 @@ describe('Component class', () => { }); it('calls an action on a component', async () => { - const { proxy, backend } = makeDummyComponent(); + const { proxy, test } = makeDummyComponent(); // @ts-ignore proxy.save({ foo: 'bar', secondArg: 'secondValue' }); // ugly: the action delays for 0ms, so we just need a TINy // delay here before we start asserting await (new Promise(resolve => setTimeout(resolve, 5))); - expect(backend.actions).toHaveLength(1); - expect(backend.actions[0].name).toBe('save'); - expect(backend.actions[0].args).toEqual({ foo: 'bar', secondArg: 'secondValue' }); + expect(test.calledActions).toHaveLength(1); + expect(test.calledActions[0].name).toBe('save'); + expect(test.calledActions[0].args).toEqual({ foo: 'bar', secondArg: 'secondValue' }); }); }); }); diff --git a/src/LiveComponent/assets/test/ValueStore.test.ts b/src/LiveComponent/assets/test/ValueStore.test.ts index 9faa081ce5d..c5a0b185ca9 100644 --- a/src/LiveComponent/assets/test/ValueStore.test.ts +++ b/src/LiveComponent/assets/test/ValueStore.test.ts @@ -337,4 +337,41 @@ describe('ValueStore', () => { expect(store.getUpdatedPropsFromParent()).toEqual(changed ? newProps : {}); }); }); + + it('returns the correctly changed models from reinitializeAllProps()', () => { + const store = new ValueStore({ + firstName: 'Ryan', + lastName: 'Weaver', + user: { + firstName: 'Ryan', + lastName: 'Weaver', + }, + product: 5, + color: 'purple', + }); + + // "dirty" props are not returned because the dirty value remains + // and overrides the new value + store.set('color', 'orange'); + + const changedProps = store.reinitializeAllProps({ + firstName: 'Ryan', + lastName: 'Bond', + user: { + firstName: 'Ryan', + lastName: 'Bond', + }, + product: { + id: 5, + name: 'cool stuff', + }, + color: 'green' + }); + + expect(changedProps).toEqual([ + 'lastName', + 'user.lastName', + 'product', + ]); + }); }); diff --git a/src/LiveComponent/assets/test/tools.ts b/src/LiveComponent/assets/test/tools.ts index d50d1678a82..4e12437f898 100644 --- a/src/LiveComponent/assets/test/tools.ts +++ b/src/LiveComponent/assets/test/tools.ts @@ -408,7 +408,7 @@ const getControllerElement = (container: HTMLElement): HTMLElement => { return element; }; -const dataToJsonAttribute = (data: any): string => { +export const dataToJsonAttribute = (data: any): string => { const container = document.createElement('div'); container.dataset.foo = JSON.stringify(data); diff --git a/src/LiveComponent/doc/index.rst b/src/LiveComponent/doc/index.rst index 5b1e4e1e75a..8386dc8f4a5 100644 --- a/src/LiveComponent/doc/index.rst +++ b/src/LiveComponent/doc/index.rst @@ -873,6 +873,8 @@ The following hooks are available (along with the arguments that are passed): * ``loading.state:started`` args ``(element: HTMLElement, request: BackendRequest)`` * ``loading.state:finished`` args ``(element: HTMLElement)`` * ``model:set`` args ``(model: string, value: any, component: Component)`` + Triggered immediately when a prop is changed via any means on the frontend + or a prop is changed on the server during a re-render/action. Adding a Stimulus Controller to your Component Root Element ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ diff --git a/src/LiveComponent/src/LiveComponentHydrator.php b/src/LiveComponent/src/LiveComponentHydrator.php index df2b7989cb8..4259570cb2b 100644 --- a/src/LiveComponent/src/LiveComponentHydrator.php +++ b/src/LiveComponent/src/LiveComponentHydrator.php @@ -257,15 +257,15 @@ private function calculateChecksum(array $dehydratedPropsData): ?string return base64_encode(hash_hmac('sha256', json_encode($dehydratedPropsData), $this->secret, true)); } - private function verifyChecksum(array $identifierPops, string $error = 'Invalid checksum sent when updating the live component.'): void + private function verifyChecksum(array $identifierProps, string $error = 'Invalid checksum sent when updating the live component.'): void { - if (!\array_key_exists(self::CHECKSUM_KEY, $identifierPops)) { + if (!\array_key_exists(self::CHECKSUM_KEY, $identifierProps)) { throw new HydrationException(sprintf('Missing %s. key', self::CHECKSUM_KEY)); } - $sentChecksum = $identifierPops[self::CHECKSUM_KEY]; - unset($identifierPops[self::CHECKSUM_KEY]); + $sentChecksum = $identifierProps[self::CHECKSUM_KEY]; + unset($identifierProps[self::CHECKSUM_KEY]); - $expectedChecksum = $this->calculateChecksum($identifierPops); + $expectedChecksum = $this->calculateChecksum($identifierProps); if (hash_equals($expectedChecksum, $sentChecksum)) { return;