mirror of
https://github.com/strapi/strapi.git
synced 2026-05-03 16:22:30 +00:00
fix(content-manager): guard cross-kind media swap from framework reconcilers
Two related fixes for the Phase 2 built-in media handler:
1. Detect React-managed DOM subtrees and skip the cross-kind DOM node
replacement. Previously the built-in called target.replaceWith(newEl)
regardless of ownership; when React later reconciled, its fiber still
pointed at the old detached node and threw
"Node.removeChild: The node to be removed is not a child of this node"
on the next render. The guard walks ancestors looking for React's
__reactFiber / __reactProps property markers and bails if any is found.
Preact and other fiber-convention frameworks are covered too.
2. Stop dispatching strapiUpdate from the admin in response to the
STRAPI_FIELD_REPLACE_UNHANDLED signal. A full-page refresh re-fetches
the server's last saved draft, wiping any in-flight live-preview
patches the user has made since their last save. The preview now
stays at its current state for unhandled changes — save to reflect
them. Integrators wanting live cross-kind behavior in framework-
managed sites should register their own onType('media', ...) handler.
The unhandled signal itself is preserved on the wire for future use
(telemetry, "save to preview" UX hints).
Adds 4 new unit tests:
- isReactManagedElement: plain, fiber-marked, ancestor-fiber-marked, null
- BUILT_IN_MEDIA_HANDLER: returns false on cross-kind when target is
React-managed; DOM is left untouched
Plan updated with the hard constraint that full-refresh fallback is not
acceptable mid-edit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -36,7 +36,7 @@ import { createYupSchema } from '../../utils/validation';
|
||||
import { InputPopover } from '../components/InputPopover';
|
||||
import { PreviewHeader } from '../components/PreviewHeader';
|
||||
import { useGetPreviewUrlQuery } from '../services/preview';
|
||||
import { INTERNAL_EVENTS, PUBLIC_EVENTS } from '../utils/constants';
|
||||
import { PUBLIC_EVENTS } from '../utils/constants';
|
||||
import { getSendMessage } from '../utils/getSendMessage';
|
||||
import { previewScript } from '../utils/previewScript';
|
||||
|
||||
@@ -159,16 +159,12 @@ const PreviewPage = () => {
|
||||
sendMessage(PUBLIC_EVENTS.STRAPI_SCRIPT, { script });
|
||||
}
|
||||
|
||||
// A field change couldn't be resolved in place and no scoped-refresh handler was
|
||||
// registered. Fall back to the existing full-page refresh (strapiUpdate) so the preview
|
||||
// stays consistent with the admin form.
|
||||
if (event.data?.type === INTERNAL_EVENTS.STRAPI_FIELD_REPLACE_UNHANDLED) {
|
||||
iframeRef.current?.contentWindow?.postMessage(
|
||||
{ type: PUBLIC_EVENTS.STRAPI_UPDATE },
|
||||
// Safe to use because the iframe origin is pinned via allowedOrigins config
|
||||
new URL(iframeRef.current.src).origin
|
||||
);
|
||||
}
|
||||
// Intentionally NOT dispatching strapiUpdate in response to STRAPI_FIELD_REPLACE_UNHANDLED:
|
||||
// a full-page refresh re-fetches the server's last-saved draft, which would wipe any
|
||||
// in-place live-preview patches the user has made since their last save. Leaving the
|
||||
// preview stale for the unhandled change is a better UX than regressing to a stale server
|
||||
// state. Integrators can ship a scoped-refresh handler via window.strapiPreview.onType()
|
||||
// for smoother behavior on cross-kind / empty-transition edits.
|
||||
};
|
||||
|
||||
window.addEventListener('message', handleMessage);
|
||||
|
||||
@@ -119,6 +119,28 @@ const previewScript = (config: PreviewScriptConfig) => {
|
||||
return root.querySelector(MEDIA_TAGS.join(','));
|
||||
};
|
||||
|
||||
/**
|
||||
* Detect whether an element (or any ancestor) is owned by React. React attaches fiber
|
||||
* references to its managed DOM nodes as properties prefixed with `__reactFiber$` and
|
||||
* `__reactProps$`. If we replace a React-managed node via replaceWith(), React's next
|
||||
* reconciliation still holds a pointer to the old node and throws
|
||||
* "Node.removeChild: The node to be removed is not a child of this node"
|
||||
* when it tries to unmount it. We use this check to skip node-replacing behaviors on
|
||||
* React-managed subtrees, falling through to the full-page-refresh fallback instead.
|
||||
*/
|
||||
const isReactManagedElement = (element: Element | null): boolean => {
|
||||
let current: Element | null = element;
|
||||
while (current) {
|
||||
for (const key in current) {
|
||||
if (key.startsWith('__reactFiber') || key.startsWith('__reactProps')) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
current = current.parentElement;
|
||||
}
|
||||
return false;
|
||||
};
|
||||
|
||||
/**
|
||||
* Resolve a media URL from the admin's payload against the current element's attribute so
|
||||
* relative paths (e.g. `/uploads/photo.jpg` from Strapi's local upload provider) keep working
|
||||
@@ -271,6 +293,18 @@ const previewScript = (config: PreviewScriptConfig) => {
|
||||
return patchMediaElement(target, value as MediaValue);
|
||||
}
|
||||
|
||||
// Cross-kind swap requires replacing the DOM node. Bail if React (or a framework using
|
||||
// the same fiber-property convention) owns the subtree — otherwise its next reconciliation
|
||||
// will try to remove the old node we already detached and throw
|
||||
// "Node.removeChild: The node to be removed is not a child of this node"
|
||||
// Returning false propagates upward to the unhandled signal. The admin intentionally does
|
||||
// NOT respond by forcing a full-page refresh — doing so would re-fetch the server's last
|
||||
// saved draft and wipe the user's in-flight edits. Integrators can ship a framework-aware
|
||||
// handler via window.strapiPreview.onType('media', ...) for this case.
|
||||
if (isReactManagedElement(target)) {
|
||||
return false;
|
||||
}
|
||||
|
||||
// Cross-kind: replace the element with a fresh one of the right tag.
|
||||
const newEl = document.createElement(desiredTag);
|
||||
const rawUrl = typeof (value as MediaValue).url === 'string' ? (value as MediaValue).url! : '';
|
||||
@@ -348,6 +382,7 @@ const previewScript = (config: PreviewScriptConfig) => {
|
||||
findMediaTarget,
|
||||
patchMediaElement,
|
||||
resolveMediaUrl,
|
||||
isReactManagedElement,
|
||||
BUILT_IN_MEDIA_HANDLER,
|
||||
resolveHandlerChain,
|
||||
runHandlerChain,
|
||||
|
||||
@@ -326,6 +326,35 @@ describe('previewScript helpers', () => {
|
||||
});
|
||||
});
|
||||
|
||||
describe('isReactManagedElement', () => {
|
||||
it('returns false for a plain element', () => {
|
||||
const el = document.createElement('div');
|
||||
expect(getHelpers().isReactManagedElement(el)).toBe(false);
|
||||
});
|
||||
|
||||
it('returns true when the element carries a React fiber property', () => {
|
||||
const el = document.createElement('img');
|
||||
(el as unknown as Record<string, unknown>).__reactFiber$xyz123 = {};
|
||||
expect(getHelpers().isReactManagedElement(el)).toBe(true);
|
||||
});
|
||||
|
||||
it('returns true when an ancestor carries a React fiber property', () => {
|
||||
const parent = document.createElement('section');
|
||||
(parent as unknown as Record<string, unknown>).__reactFiber$abc = {};
|
||||
const child = document.createElement('img');
|
||||
parent.appendChild(child);
|
||||
document.body.appendChild(parent);
|
||||
|
||||
expect(getHelpers().isReactManagedElement(child)).toBe(true);
|
||||
|
||||
document.body.removeChild(parent);
|
||||
});
|
||||
|
||||
it('returns false for a null input', () => {
|
||||
expect(getHelpers().isReactManagedElement(null)).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('BUILT_IN_MEDIA_HANDLER', () => {
|
||||
const meta = { path: 'hero.image', type: 'media' };
|
||||
|
||||
@@ -366,6 +395,30 @@ describe('previewScript helpers', () => {
|
||||
expect(wrapper.querySelector('img')).toBeNull();
|
||||
});
|
||||
|
||||
it('falls through on cross-kind when the target is React-managed', () => {
|
||||
// Simulate a React-owned <img> by attaching a fiber property. The built-in must NOT
|
||||
// call replaceWith on this node — doing so leaves React's fiber pointing at a detached
|
||||
// node and its next reconciliation throws
|
||||
// "Node.removeChild: The node to be removed is not a child of this node"
|
||||
const wrapper = document.createElement('div');
|
||||
const img = document.createElement('img');
|
||||
img.setAttribute('src', 'https://example.com/old.jpg');
|
||||
(img as unknown as Record<string, unknown>).__reactFiber$abc = {};
|
||||
wrapper.appendChild(img);
|
||||
|
||||
const handled = getHelpers().BUILT_IN_MEDIA_HANDLER(
|
||||
{ url: 'https://example.com/clip.mp4', mime: 'video/mp4' },
|
||||
wrapper,
|
||||
meta
|
||||
);
|
||||
|
||||
expect(handled).toBe(false);
|
||||
// The DOM must be untouched — the original <img> stays exactly as it was
|
||||
expect(wrapper.querySelector('video')).toBeNull();
|
||||
expect(wrapper.querySelector('img')).toBe(img);
|
||||
expect(img).toHaveAttribute('src', 'https://example.com/old.jpg');
|
||||
});
|
||||
|
||||
it('swaps <video> to <img> on video → image cross-kind change', () => {
|
||||
const wrapper = document.createElement('div');
|
||||
const video = document.createElement('video');
|
||||
|
||||
@@ -61,26 +61,29 @@ The first end-to-end vertical slice. An editor who changes the alt text of an im
|
||||
|
||||
### What to build
|
||||
|
||||
Complete the media story. Introduce the public `window.strapiPreview` API and the scoped-refresh primitive. Use both to cover the media cases Phase 1 deferred: swapping an image for a video, emptying a populated field, and populating an empty field — all without a full page reload.
|
||||
Complete the media story. Introduce the public `window.strapiPreview` API and the scoped-refresh primitive. Cover the media cases Phase 1 deferred: swapping an image for a video, emptying a populated field, and populating an empty field.
|
||||
|
||||
**Hard constraint discovered during Phase 2:** we cannot fall back to a full-page refresh when the in-place path can't handle a change. A refresh re-fetches the server's last saved draft, which would roll back any in-flight live-preview patches the user has made since their last save. Falling back silently (preview stays stale for that change until save) is the correct behavior.
|
||||
|
||||
- **Public API.** `window.strapiPreview` is exposed inside the preview iframe with `version` (numeric, starts at 1), `onType(type, handler)`, `onField(path, handler)`, and `off(key)`. Handlers registered here participate in the resolution order defined in the architectural decisions.
|
||||
- **Scoped-refresh primitive.** When a field change cannot be patched in place, the preview script invokes the resolved handler (per-field → per-type → built-in default). The handler receives the new field value and the matched wrapper element, and is responsible for replacing the wrapper's subtree. If no handler is found, the preview script sends a new internal "unhandled" message to the admin, which triggers the existing full-page refresh. This keeps the fallback safe and behavior-preserving.
|
||||
- **Built-in media default.** A default type handler for `media` is registered on `window.strapiPreview` at script start. It handles cross-kind swaps, empty↔populated transitions, and any case the Phase 1 in-place path defers. Integrators can override via `onType('media', ...)` or `onField(path, ...)`.
|
||||
- **Admin listener.** The admin listens for the new "unhandled" message and dispatches the existing `strapiUpdate` full-page refresh in response.
|
||||
- **LaunchPad.** No new code changes required — the existing `StrapiImage` marker rendering from Phase 1 is sufficient. LaunchPad exercises the new cases as part of QA.
|
||||
- **Tests.** Preview-script unit coverage is extended for the handler-resolution dispatcher and the built-in media default. No new server-side tests required.
|
||||
- **Scoped-refresh primitive.** When a field change cannot be patched in place, the preview script invokes the resolved handler (per-field → per-type → built-in default). The handler receives the new field value and the matched wrapper element, and is responsible for updating the wrapper's subtree. If no handler is found or the built-in defers, the preview script emits an internal "unhandled" message for telemetry — the admin does not full-refresh in response.
|
||||
- **Built-in media default.** A default type handler for `media` is registered on `window.strapiPreview` at script start. It handles cross-kind swaps (image↔video) via DOM node replacement, and populated→empty transitions via attribute clearing. Node replacement is **skipped on framework-managed subtrees** (e.g. React, detected via fiber-property convention) because replacing a node out from under a framework's reconciler corrupts its virtual DOM and crashes on the next render. Integrators whose sites are framework-managed can register their own `onType('media', ...)` for live cross-kind behavior.
|
||||
- **Empty→populated limitation.** With no marker element in the DOM, there is nothing for the scoped-refresh handler to target. This case falls through to "unhandled" — the preview stays at its current state until the user saves.
|
||||
- **LaunchPad.** No new code changes required — the existing `StrapiImage` marker rendering from Phase 1 is sufficient.
|
||||
- **Tests.** Preview-script unit coverage is extended for the handler-resolution dispatcher, the built-in media default, and the React-managed guard. No new server-side tests required.
|
||||
|
||||
### Acceptance criteria
|
||||
|
||||
- [ ] `window.strapiPreview` is present inside the preview iframe once the injected script runs, with a `version` of 1 and the documented method surface.
|
||||
- [ ] `onType('media', handler)` replaces the built-in default for media field changes; `off` deregisters it and restores the default.
|
||||
- [ ] `onField(path, handler)` takes precedence over `onType` for that specific path.
|
||||
- [ ] Swapping an image for a video in a media field that allows both updates the preview to the correct element type without a full reload.
|
||||
- [ ] Clearing a populated media field updates the preview to the empty state without a full reload.
|
||||
- [ ] Populating a previously empty media field updates the preview to show the new media without a full reload.
|
||||
- [ ] When no handler is registered for a field type and no built-in default covers the change, the admin receives the "unhandled" message and triggers a full-page refresh — never leaves the preview in a broken state.
|
||||
- [ ] On a non-framework-managed site, swapping an image for a video updates the DOM in place (img → video) without a full reload and with the marker preserved.
|
||||
- [ ] On a framework-managed site, the cross-kind swap is skipped and the preview stays at its current state; no crash and no rollback of other pending edits.
|
||||
- [ ] Clearing a populated media field clears the element's `src`/`srcset`/`alt`/`poster` attributes in place.
|
||||
- [ ] Populating a previously empty media field falls through to unhandled; the preview stays stale until save; the admin does not issue a full-page refresh that would discard in-flight edits.
|
||||
- [ ] `STRAPI_FIELD_REPLACE_UNHANDLED` is emitted when no handler resolves a change; the admin listens but takes no action by default (room for future UX hints like "save to preview this change").
|
||||
- [ ] All Phase 1 acceptance criteria still pass.
|
||||
- [ ] LaunchPad QA covers each of the above transitions with scroll preservation.
|
||||
- [ ] LaunchPad QA covers each of the above with no crashes and no preview rollbacks.
|
||||
|
||||
---
|
||||
|
||||
|
||||
Reference in New Issue
Block a user