From db8347f5fc73f132e85a729403c68963706ad8b6 Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Mon, 24 Aug 2026 11:42:18 -0400 Subject: [PATCH 1/2] Stop cloning the whole values tree on every publish values mutation jsonDecodeValue, jsonEncodeValue and forgetValue each cloned the entire tree, mutated the copy and reassigned it, and missingValue cloned it just to do a read-only walk. The constructor already clones, so the tree is private to the instance, and setValue was already mutating it in place. Co-Authored-By: Claude Opus 5 (1M context) --- resources/js/components/publish/Values.js | 27 +++------- resources/js/tests/PublishValues.test.js | 62 +++++++++++++++++++++++ 2 files changed, 69 insertions(+), 20 deletions(-) diff --git a/resources/js/components/publish/Values.js b/resources/js/components/publish/Values.js index d25f9001d2d..eac5e977b3a 100644 --- a/resources/js/components/publish/Values.js +++ b/resources/js/components/publish/Values.js @@ -83,7 +83,8 @@ export default class Values { missingValue(dottedKey) { var properties = Array.isArray(dottedKey) ? dottedKey : dottedKey.split('.'); - var value = properties.reduce((prev, curr) => (prev == null ? undefined : prev[curr]), clone(this.values)); + // Read-only walk — no need to clone. The constructor already made this.values private. + var value = properties.reduce((prev, curr) => (prev == null ? undefined : prev[curr]), this.values); return value === undefined; } @@ -91,25 +92,15 @@ export default class Values { jsonDecodeValue(dottedKey) { if (this.missingValue(dottedKey)) return; - let values = clone(this.values); - let fieldValue = data_get(values, dottedKey); - let decodedFieldValue = JSON.parse(fieldValue); - - data_set(values, dottedKey, decodedFieldValue); - - this.values = values; + let fieldValue = data_get(this.values, dottedKey); + data_set(this.values, dottedKey, JSON.parse(fieldValue)); } jsonEncodeValue(dottedKey) { if (this.missingValue(dottedKey)) return; - let values = clone(this.values); - let fieldValue = data_get(values, dottedKey); - let encodedFieldValue = JSON.stringify(fieldValue); - - data_set(values, dottedKey, encodedFieldValue); - - this.values = values; + let fieldValue = data_get(this.values, dottedKey); + data_set(this.values, dottedKey, JSON.stringify(fieldValue)); } setValue(dottedKey, value) { @@ -129,10 +120,6 @@ export default class Values { forgetValue(dottedKey) { if (this.missingValue(dottedKey)) return; - let values = clone(this.values); - - data_delete(values, dottedKey); - - this.values = values; + data_delete(this.values, dottedKey); } } diff --git a/resources/js/tests/PublishValues.test.js b/resources/js/tests/PublishValues.test.js index 9a6e2a6e5c4..4c5e06ed7f0 100644 --- a/resources/js/tests/PublishValues.test.js +++ b/resources/js/tests/PublishValues.test.js @@ -563,6 +563,68 @@ test('it properly sets keys that javascript considers having numeric separators' expect(newValues).toEqual(expected); }); +test('it never mutates the values it was constructed from', () => { + let values = { + first_name: 'Han', + ship: { + name: 'Falcon', + junk: true, + }, + bffs: JSON.stringify([{ name: 'Chewy', type: 'Wookie' }]), + }; + + let original = JSON.parse(JSON.stringify(values)); + let instance = new Values(values, ['bffs']); + + instance.get('bffs.0.name'); + instance.set('ship.name', 'Junker'); + instance.set('bffs.0.type', 'Beast'); + instance.jsonDecode(); + instance.forgetValue('first_name'); + instance.jsonEncode(); + instance.except(['ship.junk']); + + expect(values).toEqual(original); +}); + +test('it does not mutate the instance it merges dotted keys from', () => { + let source = new Values({ id: 'abc', title: 'Falcon' }); + let target = new Values({ id: 'xyz', title: 'X-Wing' }); + + target.mergeDottedKeys(['id'], source); + + expect(target.all()).toEqual({ id: 'abc', title: 'X-Wing' }); + expect(source.all()).toEqual({ id: 'abc', title: 'Falcon' }); +}); + +test('it does not mutate values when checking for a missing one', () => { + let instance = new Values({ ship: { name: 'Falcon' } }); + + expect(instance.missingValue('ship.crew')).toBe(true); + expect(instance.missingValue('ship.name')).toBe(false); + expect(instance.all()).toEqual({ ship: { name: 'Falcon' } }); +}); + +test('it decodes and encodes a single json value in place', () => { + let instance = new Values({ bffs: JSON.stringify([{ name: 'Chewy' }]) }, ['bffs']); + + instance.jsonDecodeValue('bffs'); + expect(instance.all()).toEqual({ bffs: [{ name: 'Chewy' }] }); + + instance.jsonEncodeValue('bffs'); + expect(instance.all()).toEqual({ bffs: JSON.stringify([{ name: 'Chewy' }]) }); +}); + +test('it leaves values alone when decoding, encoding or forgetting a missing key', () => { + let instance = new Values({ ship: { name: 'Falcon' } }); + + instance.jsonDecodeValue('ship.crew'); + instance.jsonEncodeValue('ship.crew'); + instance.forgetValue('ship.crew'); + + expect(instance.all()).toEqual({ ship: { name: 'Falcon' } }); +}); + test('it does not throw when rejecting a value nested under a null node', () => { let values = { first_name: 'Han', From 44b0dcd33ec1131dabc93a526f2ff8111a52b84f Mon Sep 17 00:00:00 2001 From: Jason Varga Date: Mon, 24 Aug 2026 11:42:22 -0400 Subject: [PATCH 2/2] Only watch the Live Preview payload while the preview is open The [payload, target] deep watcher was installed unconditionally, so every publish form deep-watched its whole values tree whether or not Live Preview was ever opened. It's now installed when the preview is enabled and torn down when it's disabled or the component unmounts. The watcher also skips posting a payload identical to the last one posted. Explicit update() callers (open / popout / refresh) still always post. Co-Authored-By: Claude Opus 5 (1M context) --- .../components/ui/LivePreview/LivePreview.vue | 84 ++++--- .../ui/LivePreview/LivePreview.test.js | 207 ++++++++++++++++++ 2 files changed, 264 insertions(+), 27 deletions(-) create mode 100644 resources/js/tests/components/ui/LivePreview/LivePreview.test.js diff --git a/resources/js/components/ui/LivePreview/LivePreview.vue b/resources/js/components/ui/LivePreview/LivePreview.vue index c10ec99ec10..275c4072bd8 100644 --- a/resources/js/components/ui/LivePreview/LivePreview.vue +++ b/resources/js/components/ui/LivePreview/LivePreview.vue @@ -66,22 +66,6 @@ const livePreviewFieldsPortal = computed(() => { return `live-preview-fields-${name.value}`; }); -watch( - () => props.enabled, - (enabled, wasEnabled) => { - if (wasEnabled && !enabled) { - nextTick(() => (portalEnabled.value = false)); - } else { - portalEnabled.value = enabled; - } - - if (!enabled) return; - - update(); - animateIn(); - }, -); - const tokenizedUrl = computed(() => { let url = props.url; @@ -98,32 +82,33 @@ const payload = computed(() => ({ extras: extras.value, })); -watch( - [payload, target], - (payload) => { - if (props.enabled) update(); - }, - { deep: true }, -); +// The payload is only watched while the preview is open, and a deep change that +// serializes to the payload we last posted doesn't warrant posting again. Explicit +// update() callers (open / popout / refresh) bypass this and always post. +let lastPostedPayloadKey = null; +let stopPayloadWatch = null; const livePreviewDebounceMs = Statamic.$config.get('livePreview.debounce_ms', 150); const update = debounce(() => { + const body = payload.value; + lastPostedPayloadKey = JSON.stringify([body, target.value]); + if (source) source.abort(); source = new AbortController(); loading.value = true; axios - .post(tokenizedUrl.value, payload.value, { signal: source.signal }) + .post(tokenizedUrl.value, body, { signal: source.signal }) .then((response) => { token.value = response.data.token; const url = response.data.url; const tgt = toRaw(props.targets[target.value]); - const payload = { token: token.value, reference: props.reference }; + const messagePayload = { token: token.value, reference: props.reference }; poppedOut.value - ? channel.value.postMessage({ event: 'updated', url, target: tgt, payload }) - : updateIframeContents(url, tgt, payload, setIframeAttributes); + ? channel.value.postMessage({ event: 'updated', url, target: tgt, payload: messagePayload }) + : updateIframeContents(url, tgt, messagePayload, setIframeAttributes); loading.value = false; }) .catch((e) => { @@ -169,6 +154,50 @@ function animateOut() { return wait(300); } +function startPayloadWatch() { + if (stopPayloadWatch) return; + + stopPayloadWatch = watch( + [payload, target], + () => { + const key = JSON.stringify([payload.value, target.value]); + if (key === lastPostedPayloadKey) return; + + update(); + }, + { deep: true }, + ); +} + +function teardownPayloadWatch() { + stopPayloadWatch?.(); + stopPayloadWatch = null; + update.cancel(); + source?.abort(); +} + +watch( + () => props.enabled, + (enabled, wasEnabled) => { + if (wasEnabled && !enabled) { + teardownPayloadWatch(); + nextTick(() => (portalEnabled.value = false)); + } else { + portalEnabled.value = enabled; + } + + if (!enabled) return; + + startPayloadWatch(); + update(); + animateIn(); + }, +); + +// The watcher above only covers transitions, so a component mounted already enabled +// needs the payload watch installed up front. +if (props.enabled) startPayloadWatch(); + const canPopOut = computed(() => typeof BroadcastChannel === 'function'); function popout() { @@ -303,6 +332,7 @@ const refreshEvent = `live-preview.${name.value}.refresh`; Statamic.$events.$on(refreshEvent, refreshHandler); onUnmounted(() => { + teardownPayloadWatch(); keybinding.value.destroy(); Statamic.$events.$off(refreshEvent, refreshHandler); }); diff --git a/resources/js/tests/components/ui/LivePreview/LivePreview.test.js b/resources/js/tests/components/ui/LivePreview/LivePreview.test.js new file mode 100644 index 00000000000..16c960d05fb --- /dev/null +++ b/resources/js/tests/components/ui/LivePreview/LivePreview.test.js @@ -0,0 +1,207 @@ +import { afterEach, beforeEach, expect, test, vi } from 'vitest'; +import { ref } from 'vue'; +import { flushPromises, mount } from '@vue/test-utils'; +import axios from 'axios'; +import LivePreview from '@/components/ui/LivePreview/LivePreview.vue'; + +vi.mock('axios', () => ({ + default: { post: vi.fn() }, +})); + +vi.mock('@/components/ui/LivePreview/ManagesIframes.js', async () => { + const { ref } = await import('vue'); + return { useIframeManager: () => ({ previousUrl: ref(null), updateIframeContents: vi.fn() }) }; +}); + +vi.mock('@ui', async () => { + const { defineComponent, inject } = await import('vue'); + const stub = defineComponent({ setup: (props, { slots }) => () => slots.default?.() }); + + return { + Select: stub, + Button: stub, + injectPublishContext: () => inject('PublishContainerContext', null), + }; +}); + +let values; +let events; + +function livePreview(props = {}) { + return mount(LivePreview, { + props: { enabled: false, url: '/preview', targets: [{ label: 'Default' }], ...props }, + global: { + provide: { + PublishContainerContext: { + name: ref('entry-form'), + blueprint: ref({ handle: 'article' }), + values, + }, + }, + stubs: { 'v-portal': true, portal: true, 'portal-target': true, Resizer: true }, + }, + }); +} + +// The debounce is configured to 0ms below, but still goes through a timer. +async function settle() { + await vi.advanceTimersByTimeAsync(1); + await flushPromises(); +} + +async function enable(wrapper) { + await wrapper.setProps({ enabled: true }); + await settle(); +} + +beforeEach(() => { + vi.useFakeTimers(); + + values = ref({ title: 'One' }); + + events = { + handlers: {}, + $on(event, handler) { + (this.handlers[event] ??= []).push(handler); + }, + $off(event, handler) { + this.handlers[event] = (this.handlers[event] ?? []).filter((h) => h !== handler); + }, + $emit(event) { + (this.handlers[event] ?? []).forEach((handler) => handler()); + }, + }; + + global.__ = (key) => key; + + global.Statamic = { + $config: { + get: (key, fallback) => + ({ + 'livePreview.debounce_ms': 0, + 'livePreview.devices': { Responsive: {} }, + 'livePreview.inputs': {}, + })[key] ?? fallback, + }, + $keys: { bindGlobal: () => ({ destroy: vi.fn() }) }, + $events: events, + }; + + axios.post.mockResolvedValue({ data: { token: 'token', url: '/preview/rendered' } }); +}); + +afterEach(() => { + vi.useRealTimers(); + vi.clearAllMocks(); +}); + +test('it does not post while disabled, even when the values change', async () => { + livePreview(); + + values.value.title = 'Two'; + await settle(); + + expect(axios.post).not.toHaveBeenCalled(); +}); + +test('it posts once when enabled', async () => { + await enable(livePreview()); + + expect(axios.post).toHaveBeenCalledTimes(1); + expect(axios.post.mock.calls[0][1]).toEqual({ + blueprint: 'article', + preview: { title: 'One' }, + extras: {}, + }); +}); + +test('it posts when the values change while enabled', async () => { + await enable(livePreview()); + + values.value.title = 'Two'; + await settle(); + + expect(axios.post).toHaveBeenCalledTimes(2); + expect(axios.post.mock.calls[1][1].preview).toEqual({ title: 'Two' }); +}); + +test('it does not post again when the values change into an identical payload', async () => { + await enable(livePreview()); + + // A new object, but one that serializes to the payload we just posted. + values.value = { title: 'One' }; + await settle(); + + expect(axios.post).toHaveBeenCalledTimes(1); +}); + +test('it posts on an explicit refresh even when the payload is unchanged', async () => { + await enable(livePreview()); + + events.$emit('live-preview.entry-form.refresh'); + await settle(); + + expect(axios.post).toHaveBeenCalledTimes(2); +}); + +test('it stops posting once disabled', async () => { + const wrapper = livePreview(); + await enable(wrapper); + + await wrapper.setProps({ enabled: false }); + await settle(); + + values.value.title = 'Two'; + await settle(); + + expect(axios.post).toHaveBeenCalledTimes(1); +}); + +test('it does not read the values tree at all while disabled', async () => { + let reads = 0; + values = ref({ + title: 'One', + get watched() { + reads++; + return 'anything'; + }, + }); + + livePreview(); + await settle(); + + const before = reads; + values.value.title = 'Two'; + await settle(); + + // Nothing should be deep-watching the tree, so the change goes unread. + expect(reads).toBe(before); +}); + +test('it aborts an in-flight request when disabled', async () => { + let signal; + axios.post.mockImplementation((url, body, config) => { + signal = config.signal; + return new Promise(() => {}); + }); + + const wrapper = livePreview(); + await enable(wrapper); + + expect(signal.aborted).toBe(false); + + await wrapper.setProps({ enabled: false }); + await settle(); + + expect(signal.aborted).toBe(true); +}); + +test('it watches the payload when mounted already enabled', async () => { + livePreview({ enabled: true }); + + values.value.title = 'Two'; + await settle(); + + expect(axios.post).toHaveBeenCalledTimes(1); + expect(axios.post.mock.calls[0][1].preview).toEqual({ title: 'Two' }); +});