diff --git a/src/LiveComponent/CHANGELOG.md b/src/LiveComponent/CHANGELOG.md index e82ca620f51..6eee85bb73a 100644 --- a/src/LiveComponent/CHANGELOG.md +++ b/src/LiveComponent/CHANGELOG.md @@ -1,5 +1,14 @@ # CHANGELOG +## 2.5.0 + +- [BEHAVIOR CHANGE] Previously, Ajax calls could happen in parallel (if + you changed a model then triggered an action before the model update Ajax + call finished, the action Ajax call would being in parallel). Now, if + an Ajax call is currently happening, any future requests will wait until + it finishes. Then, all queued changes (potentially multiple model updates + or actions) will be sent all at once on the next request. + ## 2.4.0 - [BC BREAK] Previously, the `id` attribute was used with `morphdom` as the diff --git a/src/LiveComponent/assets/src/UnsyncedInputContainer.ts b/src/LiveComponent/assets/src/UnsyncedInputContainer.ts index f545bf6b214..04b9595535e 100644 --- a/src/LiveComponent/assets/src/UnsyncedInputContainer.ts +++ b/src/LiveComponent/assets/src/UnsyncedInputContainer.ts @@ -1,6 +1,6 @@ export default class UnsyncedInputContainer { #mappedFields: Map; - #unmappedFields: Array= []; + #unmappedFields: Array = []; constructor() { this.#mappedFields = new Map(); @@ -20,14 +20,6 @@ export default class UnsyncedInputContainer { return [...this.#unmappedFields, ...this.#mappedFields.values()] } - clone(): UnsyncedInputContainer { - const container = new UnsyncedInputContainer(); - container.#mappedFields = new Map(this.#mappedFields); - container.#unmappedFields = [...this.#unmappedFields]; - - return container; - } - allMappedFields(): Map { return this.#mappedFields; } diff --git a/src/LiveComponent/assets/src/ValueStore.ts b/src/LiveComponent/assets/src/ValueStore.ts index ea8bf051cbe..15a797a6955 100644 --- a/src/LiveComponent/assets/src/ValueStore.ts +++ b/src/LiveComponent/assets/src/ValueStore.ts @@ -4,6 +4,7 @@ import { normalizeModelName } from './string_utils'; export default class { controller: LiveController; + updatedModels: string[] = []; constructor(liveController: LiveController) { this.controller = liveController; @@ -33,6 +34,7 @@ export default class { */ set(name: string, value: any): void { const normalizedName = normalizeModelName(name); + this.updatedModels.push(normalizedName); this.controller.dataValue = setDeepData(this.controller.dataValue, normalizedName, value); } @@ -49,4 +51,8 @@ export default class { asJson(): string { return JSON.stringify(this.controller.dataValue); } + + all(): any { + return this.controller.dataValue; + } } diff --git a/src/LiveComponent/assets/src/live_controller.ts b/src/LiveComponent/assets/src/live_controller.ts index d7f1b4972fc..ff22d05fb6e 100644 --- a/src/LiveComponent/assets/src/live_controller.ts +++ b/src/LiveComponent/assets/src/live_controller.ts @@ -47,28 +47,18 @@ export default class extends Controller implements LiveController { readonly debounceValue!: number; readonly hasDebounceValue: boolean; + backendRequest: BackendRequest|null; valueStore!: ValueStore; - /** - * The current "timeout" that's waiting before a model update - * triggers a re-render. - */ - renderDebounceTimeout: number | null = null; + /** Actions that are waiting to be executed */ + pendingActions: Array<{ name: string, args: Record }> = []; + /** Has anything requested a re-render? */ + isRerenderRequested = false; /** - * The current "timeout" that's waiting before an action should - * be taken. + * Current "timeout" before the pending request should be sent. */ - actionDebounceTimeout: number | null = null; - - /** - * A stack of all current AJAX Promises for re-rendering. - * - * @type {PromiseStack} - */ - renderPromiseStack = new PromiseStack(); - - isActionProcessing = false; + requestDebounceTimeout: number | null = null; pollingIntervals: NodeJS.Timer[] = []; @@ -79,7 +69,7 @@ export default class extends Controller implements LiveController { mutationObserver: MutationObserver|null = null; /** - * Input fields that have "changed", but whose model value hasn't been set yet. + * Model form fields that have "changed", but whose model value hasn't been set yet. */ unsyncedInputs!: UnsyncedInputContainer; @@ -124,6 +114,7 @@ export default class extends Controller implements LiveController { disconnect() { this._stopAllPolling(); + this.#clearRequestDebounceTimeout(); window.removeEventListener('beforeunload', this.markAsWindowUnloaded); this.element.removeEventListener('live:update-model', this.handleUpdateModelEvent); @@ -164,19 +155,10 @@ export default class extends Controller implements LiveController { const directives = parseDirectives(rawAction); directives.forEach((directive) => { - // set here so it can be delayed with debouncing below - const _executeAction = () => { - // if any normal renders are waiting to start, cancel them - // allow the action to start and finish - // this covers a case where you "blur" a field to click "save" - // the "change" event will trigger first & schedule a re-render - // then the action Ajax will start. We want to avoid the - // re-render request from starting after the debounce and - // taking precedence - this._clearWaitingDebouncedRenders(); - - this._makeRequest(directive.action, directive.named); - } + this.pendingActions.push({ + name: directive.action, + args: directive.named + }); let handled = false; directive.modifiers.forEach((modifier) => { @@ -195,15 +177,10 @@ export default class extends Controller implements LiveController { case 'debounce': { const length: number = modifier.value ? parseInt(modifier.value) : this.getDefaultDebounce(); - // clear any pending renders - if (this.actionDebounceTimeout) { - clearTimeout(this.actionDebounceTimeout); - this.actionDebounceTimeout = null; - } - - this.actionDebounceTimeout = window.setTimeout(() => { - this.actionDebounceTimeout = null; - _executeAction(); + this.#clearRequestDebounceTimeout(); + this.requestDebounceTimeout = window.setTimeout(() => { + this.requestDebounceTimeout = null; + this.#startPendingRequest(); }, length); handled = true; @@ -223,21 +200,23 @@ export default class extends Controller implements LiveController { // model change *before* sending the action if (getModelDirectiveFromInput(event.currentTarget, false)) { this.pendingActionTriggerModelElement = event.currentTarget; + this.#clearRequestDebounceTimeout(); window.setTimeout(() => { this.pendingActionTriggerModelElement = null; - _executeAction(); + this.#startPendingRequest(); }, 10); return; } - _executeAction(); + this.#startPendingRequest(); } }) } $render() { - this._makeRequest(null, {}); + this.isRerenderRequested = true; + this.#startPendingRequest(); } /** @@ -258,8 +237,6 @@ export default class extends Controller implements LiveController { const modelDirective = getModelDirectiveFromInput(element, false); if (eventName === 'input') { const modelName = modelDirective ? modelDirective.action : null; - // notify existing promises of the new modified input - this.renderPromiseStack.addModifiedElement(element, modelName); // track any inputs that are "unsynced" this.unsyncedInputs.add(element, modelName); } @@ -353,7 +330,7 @@ export default class extends Controller implements LiveController { * @param {string|null} extraModelName Another model name that this might go by in a parent component. * @param {UpdateModelOptions} options */ - $updateModel(model: string, value: any, shouldRender = true, extraModelName: string | null = null, options: UpdateModelOptions = {}) { + $updateModel(model: string, value: any, shouldRender = true, extraModelName: string|null = null, options: UpdateModelOptions = {}) { const modelName = normalizeModelName(model); const normalizedExtraModelName = extraModelName ? normalizeModelName(extraModelName) : null; @@ -391,79 +368,96 @@ export default class extends Controller implements LiveController { // the string "4" - back into an array with [id=4, title=new_title]. this.valueStore.set(modelName, value); - // now that this value is set, remove it from unsyncedInputs - // any Ajax request that starts from this moment WILL include this - this.unsyncedInputs.remove(modelName); - // skip rendering if there is an action Ajax call processing - if (shouldRender && !this.isActionProcessing) { - // clear any pending renders - this._clearWaitingDebouncedRenders(); - + if (shouldRender) { let debounce: number = this.getDefaultDebounce(); if (options.debounce !== undefined && options.debounce !== null) { debounce = options.debounce; } - this.renderDebounceTimeout = window.setTimeout(() => { - this.renderDebounceTimeout = null; - this.$render(); + this.#clearRequestDebounceTimeout(); + this.requestDebounceTimeout = window.setTimeout(() => { + this.requestDebounceTimeout = null; + this.isRerenderRequested = true; + this.#startPendingRequest(); }, debounce); } } - _makeRequest(action: string|null, args: Record) { + /** + * Makes a request to the server with all pending actions/updates, if any. + */ + #startPendingRequest(): void { + if (!this.backendRequest && (this.pendingActions.length > 0 || this.isRerenderRequested)) { + this.#makeRequest(); + } + } + + #makeRequest() { const splitUrl = this.urlValue.split('?'); let [url] = splitUrl const [, queryString] = splitUrl; const params = new URLSearchParams(queryString || ''); - if (typeof args === 'object' && Object.keys(args).length > 0) { - params.set('args', new URLSearchParams(args).toString()); - } + const actions = this.pendingActions; + this.pendingActions = []; + this.isRerenderRequested = false; + // we're making a request NOW, so no need to make another one after debouncing + this.#clearRequestDebounceTimeout(); + + // check if any unsynced inputs are now "in sync": their value matches what's in the store + // if they ARE, then they are on longer "unsynced", which means that any + // potential new values from the server *should* now be respected and used + this.unsyncedInputs.allMappedFields().forEach((element, modelName) => { + if (getValueFromInput(element, this.valueStore) === this.valueStore.get(modelName)) { + this.unsyncedInputs.remove(modelName); + } + }); const fetchOptions: RequestInit = {}; fetchOptions.headers = { 'Accept': 'application/vnd.live-component+html', }; - if (action) { - this.isActionProcessing = true; + const updatedModels = this.valueStore.updatedModels; + this.valueStore.updatedModels = []; + if (actions.length === 0 && this._willDataFitInUrl(this.valueStore.asJson(), params)) { + params.set('data', this.valueStore.asJson()); + updatedModels.forEach((model) => { + params.append('updatedModels[]', model); + }); + fetchOptions.method = 'GET'; + } else { + fetchOptions.method = 'POST'; + fetchOptions.headers['Content-Type'] = 'application/json'; + const requestData: any = { data: this.valueStore.all() }; + requestData.updatedModels = updatedModels; - url += `/${encodeURIComponent(action)}`; + if (actions.length > 0) { + // one or more ACTIONs + if (this.csrfValue) { + fetchOptions.headers['X-CSRF-TOKEN'] = this.csrfValue; + } - if (this.csrfValue) { - fetchOptions.headers['X-CSRF-TOKEN'] = this.csrfValue; - } - } + if (actions.length === 1) { + // simple, single action + requestData.args = actions[0].args; - let dataAdded = false; - if (!action) { - const dataJson = this.valueStore.asJson(); - if (this._willDataFitInUrl(dataJson, params)) { - params.set('data', dataJson); - fetchOptions.method = 'GET'; - dataAdded = true; + url += `/${encodeURIComponent(actions[0].name)}`; + } else { + url += '/_batch'; + requestData.actions = actions; + } } - } - // if GET can't be used, fallback to POST - if (!dataAdded) { - fetchOptions.method = 'POST'; - fetchOptions.body = this.valueStore.asJson(); - fetchOptions.headers['Content-Type'] = 'application/json'; + fetchOptions.body = JSON.stringify(requestData); } this._onLoadingStart(); const paramsString = params.toString(); const thisPromise = fetch(`${url}${paramsString.length > 0 ? `?${paramsString}` : ''}`, fetchOptions); - const reRenderPromise = new ReRenderPromise(thisPromise, this.unsyncedInputs.clone()); - this.renderPromiseStack.addPromise(reRenderPromise); + this.backendRequest = new BackendRequest(thisPromise); thisPromise.then(async (response) => { - if (action) { - this.isActionProcessing = false; - } - // if the response does not contain a component, render as an error const html = await response.text(); if (response.headers.get('Content-Type') !== 'application/vnd.live-component+html') { @@ -472,15 +466,10 @@ export default class extends Controller implements LiveController { return; } - // if another re-render is scheduled, do not "run it over" - if (this.renderDebounceTimeout) { - return; - } + this.#processRerender(html, response); - const isMostRecent = this.renderPromiseStack.removePromise(thisPromise); - if (isMostRecent) { - this._processRerender(html, response, reRenderPromise.unsyncedInputContainer); - } + this.backendRequest = null; + this.#startPendingRequest(); }) } @@ -489,7 +478,7 @@ export default class extends Controller implements LiveController { * * @private */ - _processRerender(html: string, response: Response, unsyncedInputContainer: UnsyncedInputContainer) { + #processRerender(html: string, response: Response) { // check if the page is navigating away if (this.isWindowUnloaded) { return; @@ -506,30 +495,30 @@ export default class extends Controller implements LiveController { return; } - if (!this._dispatchEvent('live:render', html, true, true)) { - // preventDefault() was called - return; - } - // remove the loading behavior now so that when we morphdom // "diffs" the elements, any loading differences will not cause // elements to appear different unnecessarily this._onLoadingFinish(); + if (!this._dispatchEvent('live:render', html, true, true)) { + // preventDefault() was called + return; + } + /** * If this re-render contains "mapped" fields that were updated after * the Ajax call started, then we need those "unsynced" values to * take precedence over the (out-of-date) values returned by the server. */ const modifiedModelValues: any = {}; - if (unsyncedInputContainer.allMappedFields().size > 0) { - for (const [modelName] of unsyncedInputContainer.allMappedFields()) { + if (this.unsyncedInputs.allMappedFields().size > 0) { + for (const [modelName] of this.unsyncedInputs.allMappedFields()) { modifiedModelValues[modelName] = this.valueStore.get(modelName); } } // merge/patch in the new HTML - this._executeMorphdom(html, unsyncedInputContainer.all()); + this._executeMorphdom(html, this.unsyncedInputs.all()); // reset the modified values back to their client-side version Object.keys(modifiedModelValues).forEach((modelName) => { @@ -537,13 +526,6 @@ export default class extends Controller implements LiveController { }); } - _clearWaitingDebouncedRenders() { - if (this.renderDebounceTimeout) { - clearTimeout(this.renderDebounceTimeout); - this.renderDebounceTimeout = null; - } - } - _onLoadingStart() { this._handleLoadingToggle(true); } @@ -862,16 +844,12 @@ export default class extends Controller implements LiveController { } } else { callback = () => { - this._makeRequest(actionName, {}); + this.pendingActions.push({ name: actionName, args: {}}) + this.#startPendingRequest(); } } const timer = setInterval(() => { - // if there is already an active render promise, skip the poll - if (this.renderPromiseStack.countActivePromises() > 0) { - return; - } - callback(); }, duration); this.pollingIntervals.push(timer); @@ -1094,65 +1072,21 @@ export default class extends Controller implements LiveController { }); modal.focus(); } -} - -/** - * Tracks the current "re-render" promises. - */ -class PromiseStack { - stack: Array = []; - - addPromise(reRenderPromise: ReRenderPromise) { - this.stack.push(reRenderPromise); - } - /** - * Removes the promise AND returns `true` if it is the most recent. - */ - removePromise(promise: Promise): boolean { - const index = this.#findPromiseIndex(promise); - - // promise was not found - it was removed because a new Promise - // already resolved before it - if (index === -1) { - return false; + #clearRequestDebounceTimeout() { + // clear any pending renders + if (this.requestDebounceTimeout) { + clearTimeout(this.requestDebounceTimeout); + this.requestDebounceTimeout = null; } - - // "save" whether this is the most recent or not - const isMostRecent = this.stack.length === (index + 1); - - // remove all promises starting from the oldest up through this one - this.stack.splice(0, index + 1); - - return isMostRecent; - } - - #findPromiseIndex(promise: Promise) { - return this.stack.findIndex((item) => item.promise === promise); - } - - countActivePromises(): number { - return this.stack.length; - } - - addModifiedElement(element: HTMLElement, modelName: string|null = null): void { - this.stack.forEach((reRenderPromise) => { - reRenderPromise.addModifiedElement(element, modelName); - }); } } -class ReRenderPromise { +class BackendRequest { promise: Promise; - unsyncedInputContainer: UnsyncedInputContainer; - constructor(promise: Promise, unsyncedInputContainer: UnsyncedInputContainer) { + constructor(promise: Promise) { this.promise = promise; - this.unsyncedInputContainer = unsyncedInputContainer; - } - - addModifiedElement(element: HTMLElement, modelName: string|null = null): void { - this.unsyncedInputContainer.add(element, modelName); } } diff --git a/src/LiveComponent/assets/test/controller/action.test.ts b/src/LiveComponent/assets/test/controller/action.test.ts index 90832c37120..d8103f5a54f 100644 --- a/src/LiveComponent/assets/test/controller/action.test.ts +++ b/src/LiveComponent/assets/test/controller/action.test.ts @@ -18,9 +18,35 @@ describe('LiveController Action Tests', () => { shutdownTest(); }) - it('sends an action and cancels pending (debounce) re-renders', async () => { - const test = await createTest({ comment: '', isSaved: false }, (data: any) => ` + it('sends an action and renders the result', async () => { + const test = await createTest({ comment: 'great turtles!', isSaved: false }, (data: any) => `
+ ${data.isSaved ? 'Comment Saved!' : ''} + + +
+ `); + + test.expectsAjaxCall('post') + .expectSentData({ + comment: 'great turtles!', + isSaved: false + }) + .expectActionCalled('save') + .serverWillChangeData((data: any) => { + // server marks component as "saved" + data.isSaved = true; + }) + .init(); + + getByText(test.element, 'Save').click(); + + await waitFor(() => expect(test.element).toHaveTextContent('Comment Saved!')); + }); + + it('immediately sends an action, includes debouncing model updates and cancels those debounce renders', async () => { + const test = await createTest({ comment: '', isSaved: false }, (data: any) => ` +
${data.isSaved ? 'Comment Saved!' : ''} @@ -29,10 +55,10 @@ describe('LiveController Action Tests', () => {
`); - // ONLY a post is sent, not a re-render GET + // JUST the POST request: no other GET requests test.expectsAjaxCall('post') .expectSentData({ - comment: 'great turtles!', + comment: 'great tortugas!', isSaved: false }) .expectActionCalled('save') @@ -42,10 +68,14 @@ describe('LiveController Action Tests', () => { }) .init(); - await userEvent.type(test.queryByDataModel('comment'), 'great turtles!'); + await userEvent.type(test.queryByDataModel('comment'), 'great tortugas!'); + // type immediately, still during the model debounce getByText(test.element, 'Save').click(); await waitFor(() => expect(test.element).toHaveTextContent('Comment Saved!')); + + // wait long enough for the debounced model update to happen, if it wasn't canceled + await (new Promise(resolve => setTimeout(resolve, 50))); }); it('Sends action with named args', async () => { @@ -60,7 +90,7 @@ describe('LiveController Action Tests', () => { // ONLY a post is sent, not a re-render GET test.expectsAjaxCall('post') .expectSentData({ isSaved: false }) - .expectActionCalled('sendNamedArgs', {a: 1, b: 2, c: 3}) + .expectActionCalled('sendNamedArgs', {a: '1', b: '2', c: '3'}) .serverWillChangeData((data: any) => { // server marks component as "saved" data.isSaved = true; @@ -102,9 +132,9 @@ describe('LiveController Action Tests', () => { await waitFor(() => expect(test.element).toHaveTextContent('Food: pizza')); }); - it('prevents re-render model updates while action Ajax is pending', async () => { + it('makes model updates wait until action Ajax call finishes', async () => { const test = await createTest({ comment: 'donut', isSaved: false }, (data: any) => ` -
+
${data.isSaved ? 'Comment Saved!' : ''} @@ -112,7 +142,6 @@ describe('LiveController Action Tests', () => { ${data.comment} -
`); @@ -120,13 +149,22 @@ describe('LiveController Action Tests', () => { test.expectsAjaxCall('post') .expectSentData(test.initialData) .expectActionCalled('save') - .delayResponse(1000) // longer than debounce, so updating comment could potentially send a request + .delayResponse(100) // longer than debounce, so updating comment could potentially send a request .serverWillChangeData((data: any) => { // server marks component as "saved" data.isSaved = true; }) .init(); + // the model re-render shouldn't happen until after the action ajax finishes, + // which will take 100ms. So, don't start expecting it until nearly then + // but after the model debounce + setTimeout(() => { + test.expectsAjaxCall('get') + .expectSentData({comment: 'donut holes', isSaved: true}) + .init(); + }, 75) + // save first, then type into the box getByText(test.element, 'Save').click(); await userEvent.type(test.queryByDataModel('comment'), ' holes'); @@ -134,19 +172,35 @@ describe('LiveController Action Tests', () => { 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(); + // but soon the re-render does happen await waitFor(() => expect(test.element).toHaveTextContent('donut holes')); + }); + + it('batches multiple actions together', async () => { + const test = await createTest({ isSaved: false }, (data: any) => ` +
+ ${data.isSaved ? 'Component Saved!' : ''} + + +
+ `); - // now check that model updating works again - test.expectsAjaxCall('get') - .expectSentData({comment: 'donut holes are delicious', isSaved: true}) + // 1 request with all 3 actions + test.expectsAjaxCall('post') + .expectSentData(test.initialData) + // 3 actions called + .expectActionCalled('save') + .expectActionCalled('sync', { syncAll: '1' }) + .expectActionCalled('save') + .serverWillChangeData((data: any) => { + data.isSaved = true; + }) .init(); - await userEvent.type(test.queryByDataModel('comment'), ' are delicious'); - await waitFor(() => expect(test.element).toHaveTextContent('donut holes are delicious')); + + getByText(test.element, 'Save').click(); + getByText(test.element, 'Sync').click(); + getByText(test.element, 'Save').click(); + + await waitFor(() => expect(test.element).toHaveTextContent('Component Saved!')); }); }); diff --git a/src/LiveComponent/assets/test/controller/csrf.test.ts b/src/LiveComponent/assets/test/controller/csrf.test.ts index 0c8bad26f87..d61c7d99ce2 100644 --- a/src/LiveComponent/assets/test/controller/csrf.test.ts +++ b/src/LiveComponent/assets/test/controller/csrf.test.ts @@ -30,6 +30,7 @@ describe('LiveController CSRF Tests', () => { test.expectsAjaxCall('post') .expectSentData(test.initialData) + .expectActionCalled('save') .expectHeader('X-CSRF-TOKEN', '123TOKEN') .serverWillChangeData((data: any) => { data.isSaved = true; diff --git a/src/LiveComponent/assets/test/controller/model.test.ts b/src/LiveComponent/assets/test/controller/model.test.ts index cc1509dd69a..e7e816f29a1 100644 --- a/src/LiveComponent/assets/test/controller/model.test.ts +++ b/src/LiveComponent/assets/test/controller/model.test.ts @@ -127,55 +127,6 @@ describe('LiveController data-model Tests', () => { expect(test.controller.dataValue).toEqual({name: 'Jan'}); }); - - it('only uses the most recent render call result', async () => { - const test = await createTest({ name: 'Ryan' }, (data: any) => ` -
- - - Name is: ${data.name} -
- `); - - let renderCount = 0; - test.element.addEventListener('live:render', () => { - renderCount++; - }) - - const requests: Array<{letters: string, delay: number}> = [ - { letters: 'g', delay: 650 }, - { letters: 'gu', delay: 250 }, - { letters: 'guy', delay: 150 }, - ]; - requests.forEach((request) => { - test.expectsAjaxCall('get') - .expectSentData({ name: `Ryan${request.letters}` }) - .delayResponse(request.delay) - .init(); - }); - - await userEvent.type(test.queryByDataModel('name'), 'guy', { - // This will result in this sequence: - // A) "g" starts 200ms - // B) "gu" starts 400ms - // C) "guy" starts 600ms - // D) "gu" finishes 650ms (is ignored) - // E) "guy" finishes 750ms (is used) - // F) "g" finishes 850ms (is ignored) - delay: 200 - }); - - await waitFor(() => expect(test.element).toHaveTextContent('Name is: Ryanguy')); - expect(test.queryByDataModel('name')).toHaveValue('Ryanguy'); - expect(test.controller.dataValue).toEqual({name: 'Ryanguy'}); - - // only 1 render should have ultimately occurred - expect(renderCount).toEqual(1); - }); - it('falls back to using the name attribute when no data-model is present and
is ancestor', async () => { const test = await createTest({ color: '' }, (data: any) => `
diff --git a/src/LiveComponent/assets/test/controller/poll.test.ts b/src/LiveComponent/assets/test/controller/poll.test.ts index 3bb0db72a3d..2a3e7bdd8dc 100644 --- a/src/LiveComponent/assets/test/controller/poll.test.ts +++ b/src/LiveComponent/assets/test/controller/poll.test.ts @@ -11,6 +11,7 @@ import { shutdownTest, createTest, initComponent } from '../tools'; import { waitFor } from '@testing-library/dom'; +import userEvent from '@testing-library/user-event'; describe('LiveController polling Tests', () => { afterEach(() => { @@ -111,8 +112,6 @@ describe('LiveController polling Tests', () => { }); }); - // 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) => `
@@ -185,4 +184,42 @@ describe('LiveController polling Tests', () => { timeout: 1500 }); }); + + it('waits to send the request if request is already happening', async () => { + const test = await createTest({ renderCount: 0, name: 'Ryan' }, (data: any) => ` +
+ + + Name: ${data.name} + Render count: ${data.renderCount} +
+ `); + + // First request, from typing (debouncing is set to 1ms) + test.expectsAjaxCall('get') + .expectSentData({ renderCount: 0, name: 'Ryan Weaver'}) + .serverWillChangeData((data: any) => { + data.renderCount = 1; + }) + .delayResponse(100) + .init(); + + await userEvent.type(test.queryByDataModel('name'), ' Weaver'); + + setTimeout(() => { + // first poll, should happen after 50 ms, but needs to wait the full 100 + test.expectsAjaxCall('get') + .expectSentData({renderCount: 1, name: 'Ryan Weaver'}) + .serverWillChangeData((data: any) => { + data.renderCount = 2; + }) + .init(); + }, 75) + + await waitFor(() => expect(test.element).toHaveTextContent('Render count: 1')); + await waitFor(() => expect(test.element).toHaveTextContent('Render count: 2')); + }); }); diff --git a/src/LiveComponent/assets/test/controller/render.test.ts b/src/LiveComponent/assets/test/controller/render.test.ts index dcefc6704b2..87e494f17a6 100644 --- a/src/LiveComponent/assets/test/controller/render.test.ts +++ b/src/LiveComponent/assets/test/controller/render.test.ts @@ -240,4 +240,74 @@ describe('LiveController rendering Tests', () => { // the re-render should not have happened expect(test.element).not.toHaveTextContent('Hello'); }); + + it('waits for the previous request to finish & batches changes', async () => { + const test = await createTest({ title: 'greetings', contents: '' }, (data: any) => ` +
+ + + + Title: "${data.title}" + + +
+ `); + + // expect the initial Reload request, but delay it + test.expectsAjaxCall('get') + .expectSentData(test.initialData) + .delayResponse(100) + .init(); + + getByText(test.element, 'Reload').click(); + + setTimeout(() => { + // wait 10 ms (long enough for the shortened debounce to finish and the + // Ajax request to start) and then type into the fields + userEvent.type(test.queryByDataModel('title'), '!!!'); + userEvent.type(test.queryByDataModel('contents'), 'Welcome to our test!'); + + // NOW we're expecting th 2nd request + test.expectsAjaxCall('get') + // only 1 request, both new pieces of data sent at once + .expectSentData({ + title: 'greetings!!!', + contents: 'Welcome to our test!' + }) + .init(); + }, 10); + + await waitFor(() => expect(test.element).toHaveTextContent('Title: "greetings!!!"')); + }); + + it('batches re-render requests together that occurred during debounce', async () => { + const test = await createTest({ title: 'greetings', contents: '' }, (data: any) => ` +
+ + + + Title: "${data.title}" + + +
+ `); + + // type: 50ms debounce will begin + userEvent.type(test.queryByDataModel('title'), ' to you'); + setTimeout(() => { + // wait 40 ms: not long enough for debounce, then modify this field + userEvent.type(test.queryByDataModel('contents'), 'Welcome to our test!'); + + // the ONE request will not start until 50ms (debounce) from now + // delay 40 ms before we start to expect it + setTimeout(() => { + // just one request should be made + test.expectsAjaxCall('get') + .expectSentData({ title: 'greetings to you', contents: 'Welcome to our test!'}) + .init(); + }, 40) + }, 40); + + await waitFor(() => expect(test.element).toHaveTextContent('Title: "greetings to you"')); + }); }); diff --git a/src/LiveComponent/assets/test/tools.ts b/src/LiveComponent/assets/test/tools.ts index 4f40c4936c5..b0c01a6b22b 100644 --- a/src/LiveComponent/assets/test/tools.ts +++ b/src/LiveComponent/assets/test/tools.ts @@ -32,7 +32,8 @@ export function shutdownTest() { unmatchedFetchErrors.forEach((unmatchedFetchError) => { const urlParams = new URLSearchParams(unmatchedFetchError.url.substring(unmatchedFetchError.url.indexOf('?'))); const requestInfo = []; - requestInfo.push(` METHOD: ${unmatchedFetchError.method}`); + requestInfo.push(` URL: ${unmatchedFetchError.url}`) + requestInfo.push(` METHOD: ${unmatchedFetchError.method}`); requestInfo.push(` HEADERS: ${JSON.stringify(unmatchedFetchError.headers)}`); requestInfo.push(` DATA: ${unmatchedFetchError.method === 'GET' ? urlParams.get('data') : unmatchedFetchError.body}`); @@ -112,8 +113,7 @@ class MockedAjaxCall { method: string; test: FunctionalTest; expectedSentData?: any; - expectedActionName?: string; - expectedActionArgs: any = null; + expectedActions: Array<{ name: string, args: any }> = []; expectedHeaders: any = {}; changeDataCallback?: (data: any) => void; template?: (data: any) => string @@ -156,10 +156,12 @@ class MockedAjaxCall { return this; } - expectActionCalled(actionName: string, args: any = null): MockedAjaxCall { + expectActionCalled(actionName: string, args: any = {}): MockedAjaxCall { this.checkInitialization('expectActionName'); - this.expectedActionName = actionName; - this.expectedActionArgs = args; + this.expectedActions.push({ + name: actionName, + args: args + }) return this; } @@ -228,16 +230,20 @@ class MockedAjaxCall { getVisualSummary(): string { const requestInfo = []; - requestInfo.push(` METHOD: ${this.method}`); + if (this.method === 'GET') { + requestInfo.push(` URL MATCH: ${this.getMockMatcher().url}`); + } + requestInfo.push(` METHOD: ${this.method}`); if (Object.keys(this.expectedHeaders).length > 0) { requestInfo.push(` HEADERS: ${JSON.stringify(this.expectedHeaders)}`); } - requestInfo.push(` DATA: ${JSON.stringify(this.expectedSentData)}`); - if (this.expectedActionName) { - requestInfo.push(` Expected URL to contain action /${this.expectedActionName}`) - if (this.expectedActionArgs) { - requestInfo.push(` Expected action arguments in URL matching ${this.calculateArgsQueryString()}`) - } + if (this.method === 'GET') { + requestInfo.push(` DATA: ${JSON.stringify(this.expectedSentData)}`); + } else { + requestInfo.push(` DATA: ${JSON.stringify(this.getRequestBody())}`); + } + if (this.expectedActions.length === 1) { + requestInfo.push(` Expected URL to contain action /${this.expectedActions[0].name}`) } return requestInfo.join("\n"); @@ -259,25 +265,23 @@ class MockedAjaxCall { const params = new URLSearchParams({ data: JSON.stringify(this.expectedSentData) }); - matcherObject.url = `end:?${params.toString()}`; + matcherObject.functionMatcher = (url: string) => { + return url.includes(`?${params.toString()}`); + }; } else { - matcherObject.body = this.expectedSentData; - if (this.expectedActionName) { - matcherObject.functionMatcher = (url: string) => { - // match the "/actionName" part in the URL - if (!url.match(new RegExp(`/${this.expectedActionName}?`))) { - return false; - } - - // look for action arguments - if (this.expectedActionArgs) { - if (!url.includes(this.calculateArgsQueryString())) { - return false; - } - } - - return true; - } + // match the body, by without "updatedModels" which is not important + // and also difficult/tedious to always assert + matcherObject.functionMatcher = (url: string, request: any) => { + const body = JSON.parse(request.body); + delete body.updatedModels; + + return JSON.stringify(body) === JSON.stringify(this.getRequestBody()); + }; + + if (this.expectedActions.length === 1) { + matcherObject.url = `end:/${this.expectedActions[0].name}`; + } else if (this.expectedActions.length > 1) { + matcherObject.url = `end:/_batch`; } } @@ -287,11 +291,22 @@ class MockedAjaxCall { return matcherObject; } - private calculateArgsQueryString(): string { - const urlParams = new URLSearchParams(); - urlParams.set('args', new URLSearchParams(this.expectedActionArgs).toString()); + private getRequestBody(): any { + if (this.method === 'GET') { + return null; + } + + const body: any = { + data: this.expectedSentData + }; + + if (this.expectedActions.length === 1) { + body.args = this.expectedActions[0].args; + } else if (this.expectedActions.length > 1) { + body.actions = this.expectedActions; + } - return urlParams.toString(); + return body; } private checkInitialization = (method: string): void => { diff --git a/src/LiveComponent/src/Controller/BatchActionController.php b/src/LiveComponent/src/Controller/BatchActionController.php new file mode 100644 index 00000000000..61d65a32166 --- /dev/null +++ b/src/LiveComponent/src/Controller/BatchActionController.php @@ -0,0 +1,52 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\UX\LiveComponent\Controller; + +use Symfony\Component\HttpFoundation\Request; +use Symfony\Component\HttpFoundation\Response; +use Symfony\Component\HttpKernel\Exception\BadRequestHttpException; +use Symfony\Component\HttpKernel\HttpKernelInterface; +use Symfony\UX\TwigComponent\MountedComponent; + +/** + * @author Kevin Bond + * + * @internal + */ +final class BatchActionController +{ + public function __construct(private HttpKernelInterface $kernel) + { + } + + public function __invoke(Request $request, MountedComponent $_mounted_component, string $serviceId, array $actions): ?Response + { + foreach ($actions as $action) { + $name = $action['name'] ?? throw new BadRequestHttpException('Invalid JSON'); + + $subRequest = $request->duplicate(attributes: [ + '_controller' => [$serviceId, $name], + '_component_action_args' => $action['args'] ?? [], + '_mounted_component' => $_mounted_component, + '_route' => 'live_component', + ]); + + $response = $this->kernel->handle($subRequest, HttpKernelInterface::SUB_REQUEST, false); + + if ($response->isRedirection()) { + return $response; + } + } + + return null; + } +} diff --git a/src/LiveComponent/src/DependencyInjection/LiveComponentExtension.php b/src/LiveComponent/src/DependencyInjection/LiveComponentExtension.php index 946886ca3b6..a983c6dbcfa 100644 --- a/src/LiveComponent/src/DependencyInjection/LiveComponentExtension.php +++ b/src/LiveComponent/src/DependencyInjection/LiveComponentExtension.php @@ -20,6 +20,7 @@ use Symfony\UX\LiveComponent\Attribute\AsLiveComponent; use Symfony\UX\LiveComponent\ComponentValidator; use Symfony\UX\LiveComponent\ComponentValidatorInterface; +use Symfony\UX\LiveComponent\Controller\BatchActionController; use Symfony\UX\LiveComponent\EventListener\AddLiveAttributesSubscriber; use Symfony\UX\LiveComponent\EventListener\LiveComponentSubscriber; use Symfony\UX\LiveComponent\Form\Type\LiveCollectionType; @@ -70,6 +71,13 @@ function (ChildDefinition $definition, AsLiveComponent $attribute) { ]) ; + $container->register('ux.live_component.batch_action_controller', BatchActionController::class) + ->setPublic(true) + ->setArguments([ + new Reference('http_kernel'), + ]) + ; + $container->register('ux.live_component.event_subscriber', LiveComponentSubscriber::class) ->addTag('kernel.event_subscriber') ->addTag('container.service_subscriber', ['key' => ComponentFactory::class, 'id' => 'ux.twig_component.component_factory']) diff --git a/src/LiveComponent/src/EventListener/LiveComponentSubscriber.php b/src/LiveComponent/src/EventListener/LiveComponentSubscriber.php index 9c38f70d018..fbd1250607a 100644 --- a/src/LiveComponent/src/EventListener/LiveComponentSubscriber.php +++ b/src/LiveComponent/src/EventListener/LiveComponentSubscriber.php @@ -69,6 +69,10 @@ public function onKernelRequest(RequestEvent $event): void return; } + if ($request->attributes->has('_controller')) { + return; + } + // the default "action" is get, which does nothing $action = $request->get('action', 'get'); $componentName = (string) $request->get('component'); @@ -107,6 +111,23 @@ public function onKernelRequest(RequestEvent $event): void throw new BadRequestHttpException('Invalid CSRF token.'); } + if ('_batch' === $action) { + // use batch controller + $data = $this->parseDataFor($request); + + $request->attributes->set('_controller', 'ux.live_component.batch_action_controller'); + $request->attributes->set('serviceId', $metadata->getServiceId()); + $request->attributes->set('actions', $data['actions']); + $request->attributes->set('_mounted_component', $this->container->get(LiveComponentHydrator::class)->hydrate( + $this->container->get(ComponentFactory::class)->get($componentName), + $data['data'], + $componentName, + )); + $request->attributes->set('_is_live_batch_action', true); + + return; + } + $request->attributes->set('_controller', sprintf('%s::%s', $metadata->getServiceId(), $action)); } @@ -118,15 +139,13 @@ public function onKernelController(ControllerEvent $event): void return; } - if ($request->query->has('data')) { - // ?data= - $data = json_decode($request->query->get('data'), true, 512, \JSON_THROW_ON_ERROR); - } else { - // OR body of the request is JSON - $data = json_decode($request->getContent(), true, 512, \JSON_THROW_ON_ERROR); + if ($request->attributes->get('_is_live_batch_action')) { + return; } - if (!\is_array($controller = $event->getController()) || 2 !== \count($controller)) { + $controller = $event->getController(); + + if (!\is_array($controller) || 2 !== \count($controller)) { throw new \RuntimeException('Not a valid live component.'); } @@ -140,27 +159,66 @@ public function onKernelController(ControllerEvent $event): void throw new NotFoundHttpException(sprintf('The action "%s" either doesn\'t exist or is not allowed in "%s". Make sure it exist and has the LiveAction attribute above it.', $action, \get_class($component))); } - $mounted = $this->container->get(LiveComponentHydrator::class)->hydrate( - $component, - $data, - $request->attributes->get('_component_name') - ); - - $request->attributes->set('_mounted_component', $mounted); - - if (!\is_string($queryString = $request->query->get('args'))) { - return; + /* + * Either we: + * A) We do NOT have a _mounted_component, so hydrate $component + * B) We DO have a _mounted_component, so no need to hydrate, + * but we DO need to make sure it's set as the controller. + */ + if (!$request->attributes->has('_mounted_component')) { + $request->attributes->set('_mounted_component', $this->container->get(LiveComponentHydrator::class)->hydrate( + $component, + $this->parseDataFor($request)['data'], + $request->attributes->get('_component_name') + )); + } else { + // override the component with our already-mounted version + $component = $request->attributes->get('_mounted_component')->getComponent(); + $event->setController([ + $component, + $action, + ]); } + // read the action arguments from the request, unless they're already set (batch sub-requests) + $actionArguments = $request->attributes->get('_component_action_args', $this->parseDataFor($request)['args']); // extra variables to be made available to the controller // (for "actions" only) - parse_str($queryString, $args); - foreach (LiveArg::liveArgs($component, $action) as $parameter => $arg) { - if (isset($args[$arg])) { - $request->attributes->set($parameter, $args[$arg]); + if (isset($actionArguments[$arg])) { + $request->attributes->set($parameter, $actionArguments[$arg]); + } + } + } + + /** + * @return array{ + * data: array, + * args: array, + * actions: array + * } + */ + private function parseDataFor(Request $request): array + { + if (!$request->attributes->has('_live_request_data')) { + if ($request->query->has('data')) { + return [ + 'data' => json_decode($request->query->get('data'), true, 512, \JSON_THROW_ON_ERROR), + 'args' => [], + 'actions' => [], + ]; } + + $requestData = $request->toArray(); + + $request->attributes->set('_live_request_data', [ + 'data' => $requestData['data'] ?? [], + 'args' => $requestData['args'] ?? [], + 'actions' => $requestData['actions'] ?? [], + ]); } + + return $request->attributes->get('_live_request_data'); } public function onKernelView(ViewEvent $event): void @@ -169,6 +227,13 @@ public function onKernelView(ViewEvent $event): void return; } + if (!$event->isMainRequest()) { + // sub-request, so skip rendering + $event->setResponse(new Response()); + + return; + } + $event->setResponse($this->createResponse($request->attributes->get('_mounted_component'))); } diff --git a/src/LiveComponent/tests/Fixtures/Component/WithActions.php b/src/LiveComponent/tests/Fixtures/Component/WithActions.php new file mode 100644 index 00000000000..6bd75ef12e5 --- /dev/null +++ b/src/LiveComponent/tests/Fixtures/Component/WithActions.php @@ -0,0 +1,51 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\UX\LiveComponent\Tests\Fixtures\Component; + +use Symfony\Component\HttpFoundation\RedirectResponse; +use Symfony\Component\Routing\Generator\UrlGeneratorInterface; +use Symfony\UX\LiveComponent\Attribute\AsLiveComponent; +use Symfony\UX\LiveComponent\Attribute\LiveAction; +use Symfony\UX\LiveComponent\Attribute\LiveArg; +use Symfony\UX\LiveComponent\Attribute\LiveProp; +use Symfony\UX\LiveComponent\DefaultActionTrait; + +#[AsLiveComponent('with_actions')] +final class WithActions +{ + use DefaultActionTrait; + + #[LiveProp] + public array $items = ['initial']; + + #[LiveAction] + public function add(#[LiveArg] string $what, UrlGeneratorInterface $router): void + { + $this->items[] = $what; + } + + #[LiveAction] + public function redirect(UrlGeneratorInterface $router): RedirectResponse + { + return new RedirectResponse($router->generate('homepage')); + } + + #[LiveAction] + public function exception(): void + { + throw new \RuntimeException('Exception message'); + } + + public function nonLive(): void + { + } +} diff --git a/src/LiveComponent/tests/Fixtures/templates/components/with_actions.html.twig b/src/LiveComponent/tests/Fixtures/templates/components/with_actions.html.twig new file mode 100644 index 00000000000..138e09c8605 --- /dev/null +++ b/src/LiveComponent/tests/Fixtures/templates/components/with_actions.html.twig @@ -0,0 +1,5 @@ + + {% for item in items %} +
  • {{ item }}
    • + {% endfor %} + diff --git a/src/LiveComponent/tests/Functional/Controller/BatchActionControllerTest.php b/src/LiveComponent/tests/Functional/Controller/BatchActionControllerTest.php new file mode 100644 index 00000000000..53a0f34a822 --- /dev/null +++ b/src/LiveComponent/tests/Functional/Controller/BatchActionControllerTest.php @@ -0,0 +1,157 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace Symfony\UX\LiveComponent\Tests\Functional\Controller; + +use Symfony\Bundle\FrameworkBundle\Test\KernelTestCase; +use Symfony\UX\LiveComponent\Tests\LiveComponentTestHelper; +use Zenstruck\Browser\KernelBrowser; +use Zenstruck\Browser\Response\HtmlResponse; +use Zenstruck\Browser\Test\HasBrowser; + +/** + * @author Kevin Bond + */ +final class BatchActionControllerTest extends KernelTestCase +{ + use HasBrowser; + use LiveComponentTestHelper; + + public function testCanBatchActions(): void + { + $dehydrated = $this->dehydrateComponent($this->mountComponent('with_actions')); + + $this->browser() + ->throwExceptions() + ->get('/_components/with_actions', ['json' => ['data' => $dehydrated]]) + ->assertSuccessful() + ->assertSee('initial') + ->use(function (HtmlResponse $response, KernelBrowser $browser) { + $browser->post('/_components/with_actions/add', [ + 'json' => [ + 'data' => json_decode($response->crawler()->filter('ul')->first()->attr('data-live-data-value')), + 'args' => ['what' => 'first'], + ], + 'headers' => ['X-CSRF-TOKEN' => $response->crawler()->filter('ul')->first()->attr('data-live-csrf-value')], + ]); + }) + ->assertSee('initial') + ->assertSee('first') + ->use(function (HtmlResponse $response, KernelBrowser $browser) { + $browser->post('/_components/with_actions/_batch', [ + 'json' => [ + 'data' => json_decode($response->crawler()->filter('ul')->first()->attr('data-live-data-value')), + 'actions' => [ + ['name' => 'add', 'args' => ['what' => 'second']], + ['name' => 'add', 'args' => ['what' => 'third']], + ['name' => 'add', 'args' => ['what' => 'fourth']], + ], + ], + 'headers' => ['X-CSRF-TOKEN' => $response->crawler()->filter('ul')->first()->attr('data-live-csrf-value')], + ]); + }) + ->assertSee('initial') + ->assertSee('first') + ->assertSee('second') + ->assertSee('third') + ->assertSee('fourth') + ; + } + + public function testCsrfTokenIsChecked(): void + { + $dehydrated = $this->dehydrateComponent($this->mountComponent('with_actions')); + + $this->browser() + ->post('/_components/with_actions/_batch', ['json' => [ + 'data' => $dehydrated, + 'actions' => [], + ]]) + ->assertStatus(400) + ; + } + + public function testRedirect(): void + { + $dehydrated = $this->dehydrateComponent($this->mountComponent('with_actions')); + + $this->browser() + ->throwExceptions() + ->get('/_components/with_actions', ['json' => ['data' => $dehydrated]]) + ->assertSuccessful() + ->interceptRedirects() + ->use(function (HtmlResponse $response, KernelBrowser $browser) { + $browser->post('/_components/with_actions/_batch', [ + 'json' => [ + 'data' => json_decode($response->crawler()->filter('ul')->first()->attr('data-live-data-value')), + 'actions' => [ + ['name' => 'add', 'args' => ['what' => 'second']], + ['name' => 'redirect'], + ['name' => 'add', 'args' => ['what' => 'fourth']], + ], + ], + 'headers' => ['X-CSRF-TOKEN' => $response->crawler()->filter('ul')->first()->attr('data-live-csrf-value')], + ]); + }) + ->assertRedirectedTo('/') + ; + } + + public function testException(): void + { + $dehydrated = $this->dehydrateComponent($this->mountComponent('with_actions')); + + $this->browser() + ->get('/_components/with_actions', ['json' => ['data' => $dehydrated]]) + ->assertSuccessful() + ->use(function (HtmlResponse $response, KernelBrowser $browser) { + $browser->post('/_components/with_actions/_batch', [ + 'json' => [ + 'data' => json_decode($response->crawler()->filter('ul')->first()->attr('data-live-data-value')), + 'actions' => [ + ['name' => 'add', 'args' => ['what' => 'second']], + ['name' => 'exception'], + ['name' => 'add', 'args' => ['what' => 'fourth']], + ], + ], + 'headers' => ['X-CSRF-TOKEN' => $response->crawler()->filter('ul')->first()->attr('data-live-csrf-value')], + ]); + }) + ->assertStatus(500) + ->assertContains('Exception message') + ; + } + + public function testCannotBatchWithNonLiveAction(): void + { + $dehydrated = $this->dehydrateComponent($this->mountComponent('with_actions')); + + $this->browser() + ->get('/_components/with_actions', ['json' => ['data' => $dehydrated]]) + ->assertSuccessful() + ->use(function (HtmlResponse $response, KernelBrowser $browser) { + $browser->post('/_components/with_actions/_batch', [ + 'json' => [ + 'data' => json_decode($response->crawler()->filter('ul')->first()->attr('data-live-data-value')), + 'actions' => [ + ['name' => 'add', 'args' => ['what' => 'second']], + ['name' => 'nonLive'], + ['name' => 'add', 'args' => ['what' => 'fourth']], + ], + ], + 'headers' => ['X-CSRF-TOKEN' => $response->crawler()->filter('ul')->first()->attr('data-live-csrf-value')], + ]); + }) + ->assertStatus(404) + ->assertContains('The action \"nonLive\" either doesn\'t exist or is not allowed') + ; + } +} diff --git a/src/LiveComponent/tests/Functional/EventListener/LiveComponentSubscriberTest.php b/src/LiveComponent/tests/Functional/EventListener/LiveComponentSubscriberTest.php index d84c704b141..9c9a2b549fc 100644 --- a/src/LiveComponent/tests/Functional/EventListener/LiveComponentSubscriberTest.php +++ b/src/LiveComponent/tests/Functional/EventListener/LiveComponentSubscriberTest.php @@ -72,7 +72,7 @@ public function testCanExecuteComponentAction(): void }) ->post('/_components/component2/increase', [ 'headers' => ['X-CSRF-TOKEN' => $token], - 'body' => json_encode($dehydrated), + 'body' => json_encode(['data' => $dehydrated]), ]) ->assertSuccessful() ->assertHeaderContains('Content-Type', 'html') @@ -145,7 +145,7 @@ public function testDisabledCsrfTokenForComponentDoesNotFail(): void ->assertHeaderContains('Content-Type', 'html') ->assertContains('Count: 1') ->post('/_components/disabled_csrf/increase', [ - 'body' => json_encode($dehydrated), + 'body' => json_encode(['data' => $dehydrated]), ]) ->assertSuccessful() ->assertHeaderContains('Content-Type', 'html') @@ -184,7 +184,7 @@ public function testCanRedirectFromComponentAction(): void // with no custom header, it redirects like a normal browser ->post('/_components/component2/redirect', [ 'headers' => ['X-CSRF-TOKEN' => $token], - 'body' => json_encode($dehydrated), + 'body' => json_encode(['data' => $dehydrated]), ]) ->assertRedirectedTo('/') @@ -194,7 +194,7 @@ public function testCanRedirectFromComponentAction(): void 'Accept' => 'application/vnd.live-component+html', 'X-CSRF-TOKEN' => $token, ], - 'body' => json_encode($dehydrated), + 'body' => json_encode(['data' => $dehydrated]), ]) ->assertStatus(204) ->assertHeaderEquals('Location', '/') @@ -206,10 +206,10 @@ public function testInjectsLiveArgs(): void $dehydrated = $this->dehydrateComponent($this->mountComponent('component6')); $token = null; - $argsQueryParams = http_build_query(['args' => http_build_query(['arg1' => 'hello', 'arg2' => 666, 'custom' => '33.3'])]); + $arguments = ['arg1' => 'hello', 'arg2' => 666, 'custom' => '33.3']; $this->browser() ->throwExceptions() - ->get('/_components/component6?data='.urlencode(json_encode($dehydrated)).'&'.$argsQueryParams) + ->get('/_components/component6?data='.urlencode(json_encode($dehydrated))) ->assertSuccessful() ->assertHeaderContains('Content-Type', 'html') ->assertContains('Arg1: not provided') @@ -219,9 +219,12 @@ public function testInjectsLiveArgs(): void // get a valid token to use for actions $token = $response->crawler()->filter('div')->first()->attr('data-live-csrf-value'); }) - ->post('/_components/component6/inject?'.$argsQueryParams, [ + ->post('/_components/component6/inject', [ 'headers' => ['X-CSRF-TOKEN' => $token], - 'body' => json_encode($dehydrated), + 'body' => json_encode([ + 'data' => $dehydrated, + 'args' => $arguments, + ]), ]) ->assertSuccessful() ->assertHeaderContains('Content-Type', 'html') diff --git a/src/LiveComponent/tests/Functional/Form/ComponentWithFormTest.php b/src/LiveComponent/tests/Functional/Form/ComponentWithFormTest.php index cfae34c847c..3def525834f 100644 --- a/src/LiveComponent/tests/Functional/Form/ComponentWithFormTest.php +++ b/src/LiveComponent/tests/Functional/Form/ComponentWithFormTest.php @@ -50,7 +50,7 @@ public function testFormValuesRebuildAfterFormChanges(): void // post to action, which will add a new embedded comment ->post('/_components/form_with_collection_type/addComment', [ - 'body' => json_encode($dehydrated), + 'body' => json_encode(['data' => $dehydrated]), 'headers' => ['X-CSRF-TOKEN' => $token], ]) ->assertStatus(422) @@ -85,8 +85,8 @@ public function testFormValuesRebuildAfterFormChanges(): void }) // post to action, which will remove the original embedded comment - ->post('/_components/form_with_collection_type/removeComment?'.http_build_query(['args' => 'index=0']), [ - 'body' => json_encode($dehydrated), + ->post('/_components/form_with_collection_type/removeComment', [ + 'body' => json_encode(['data' => $dehydrated, 'args' => ['index' => '0']]), 'headers' => ['X-CSRF-TOKEN' => $token], ]) ->assertStatus(422) @@ -265,8 +265,8 @@ public function testLiveCollectionTypeFieldsAddedAndRemoved(): void $token = $response->crawler()->filter('div')->first()->attr('data-live-csrf-value'); }) // post to action, which will add a new embedded comment - ->post('/_components/form_with_live_collection_type/addCollectionItem?'.http_build_query(['args' => 'name=blog_post_form[comments]']), [ - 'body' => json_encode($dehydrated), + ->post('/_components/form_with_live_collection_type/addCollectionItem', [ + 'body' => json_encode(['data' => $dehydrated, 'args' => ['name' => 'blog_post_form[comments]']]), 'headers' => ['X-CSRF-TOKEN' => $token], ]) ->assertStatus(422) @@ -301,8 +301,8 @@ public function testLiveCollectionTypeFieldsAddedAndRemoved(): void }) // post to action, which will remove the original embedded comment - ->post('/_components/form_with_live_collection_type/removeCollectionItem?'.http_build_query(['args' => 'name=blog_post_form[comments]&index=0']), [ - 'body' => json_encode($dehydrated), + ->post('/_components/form_with_live_collection_type/removeCollectionItem', [ + 'body' => json_encode(['data' => $dehydrated, 'args' => ['name' => 'blog_post_form[comments]', 'index' => '0']]), 'headers' => ['X-CSRF-TOKEN' => $token], ]) ->assertStatus(422)