From 9552b9953b9706668e5c387ee5763d2dd4a0bf73 Mon Sep 17 00:00:00 2001 From: Tung Lam <53996158+tunglambk@users.noreply.github.com> Date: Mon, 12 Oct 2026 01:50:39 +0000 Subject: [PATCH] fix: keep effect cleanup errors inside the preview LivePreview keyed its error boundary on the code, so editing the snippet remounted the boundary in the same commit that unmounted the element it was meant to catch. A snippet whose useEffect returned a non-function (React.useEffect(() => [])) then threw during cleanup with no boundary left to catch it, and the error ran up to the app's root boundary and took the page down. The boundary now stays mounted and resets its error state from the code through a resetKey prop. --- .changeset/spotty-melons-listen.md | 5 ++ .../components/Live/ErrorBoundary.test.jsx | 54 +++++++++++++++++-- .../src/components/Live/ErrorBoundary.tsx | 11 +++- .../src/components/Live/LivePreview.tsx | 2 +- 4 files changed, 67 insertions(+), 5 deletions(-) create mode 100644 .changeset/spotty-melons-listen.md diff --git a/.changeset/spotty-melons-listen.md b/.changeset/spotty-melons-listen.md new file mode 100644 index 00000000..36b5c841 --- /dev/null +++ b/.changeset/spotty-melons-listen.md @@ -0,0 +1,5 @@ +--- +"react-live": patch +--- + +Keep an effect cleanup error from a replaced snippet inside the preview. `LivePreview`'s error boundary was keyed on the code, so changing the code remounted it in the same commit that unmounted the element it was meant to catch. A snippet whose `useEffect` returned something other than a function (`React.useEffect(() => [])`) threw there uncatchably, and the error reached the app's root boundary and took the page down instead of showing up in `LiveError`. The boundary now stays mounted and resets its error state through a `resetKey` prop. diff --git a/packages/react-live/src/components/Live/ErrorBoundary.test.jsx b/packages/react-live/src/components/Live/ErrorBoundary.test.jsx index f08cf162..8b1fca65 100644 --- a/packages/react-live/src/components/Live/ErrorBoundary.test.jsx +++ b/packages/react-live/src/components/Live/ErrorBoundary.test.jsx @@ -1,9 +1,11 @@ -import { render, screen } from "@testing-library/react"; +import { useContext, useEffect } from "react"; +import { act, render, screen } from "@testing-library/react"; import { ErrorBoundary } from "./ErrorBoundary"; import LiveProvider from "./LiveProvider"; import LivePreview from "./LivePreview"; import LiveError from "./LiveError"; +import LiveContext from "./LiveContext"; const Boom = () => { throw new Error("render exploded"); @@ -68,10 +70,56 @@ describe("ErrorBoundary", () => { expect(await screen.findByText(/runtime boom/)).toBeDefined(); }); + /** + * #413. `() => []` is a valid effect callback that returns an array, so React + * only trips over it when it runs the returned value as the cleanup -- which + * happens when the preview element is replaced by an edit. The boundary used + * to be keyed on the code, so it was removed in the same commit as the element + * it was meant to catch, and the throw ran all the way up to the app's root + * boundary. Here the outer ErrorBoundary stands in for that root boundary: if + * the throw reaches it the whole page is gone, not just the preview. + * + * `LiveEditor`'s onChange is what a paste calls; jsdom has no contentEditable + * editing model, so the edit is driven through the same callback it wires up. + */ + it("keeps an effect cleanup error inside the preview when the code changes", async () => { + const onPageError = vi.fn(); + let changeCode; + const CaptureOnChange = () => { + const { onChange } = useContext(LiveContext); + useEffect(() => { + changeCode = onChange; + }, [onChange]); + return null; + }; + + render( + + + + + + + , + ); + expect(await screen.findByRole("heading", { name: "first" })).toBeDefined(); + + await act(async () => changeCode("() => { React.useEffect(() => []) }")); + await act(async () => changeCode("

second

")); + + expect(onPageError).not.toHaveBeenCalled(); + expect((await screen.findByTestId("live-error")).textContent).toMatch( + /destroy is not a function/, + ); + + await act(async () => changeCode("

third

")); + expect(await screen.findByRole("heading", { name: "third" })).toBeDefined(); + }); + /** * `getDerivedStateFromError` makes `hasError` sticky for the lifetime of the - * boundary instance. That is only safe because a new boundary class is built - * on every transpile, so a fixed snippet gets a fresh one. + * boundary instance, so `LivePreview` clears it from the new code through + * `resetKey` rather than remounting the boundary (see #413). */ it("recovers once erroring code is fixed", async () => { const ui = (code) => ( diff --git a/packages/react-live/src/components/Live/ErrorBoundary.tsx b/packages/react-live/src/components/Live/ErrorBoundary.tsx index b949d05e..98bf6134 100644 --- a/packages/react-live/src/components/Live/ErrorBoundary.tsx +++ b/packages/react-live/src/components/Live/ErrorBoundary.tsx @@ -3,20 +3,29 @@ import { Component, ReactNode } from "react"; type Props = { children: ReactNode; onError?: (error: Error) => void; + resetKey?: string; }; type State = { hasError: boolean; + resetKey?: string; }; export class ErrorBoundary extends Component { + static getDerivedStateFromProps({ resetKey }: Props, state: State) { + if (resetKey !== state.resetKey) { + return { hasError: false, resetKey }; + } + return null; + } + static getDerivedStateFromError() { return { hasError: true }; } constructor(props: Props) { super(props); - this.state = { hasError: false }; + this.state = { hasError: false, resetKey: props.resetKey }; } componentDidCatch(err: Error): void { diff --git a/packages/react-live/src/components/Live/LivePreview.tsx b/packages/react-live/src/components/Live/LivePreview.tsx index 1533b15c..970f516c 100644 --- a/packages/react-live/src/components/Live/LivePreview.tsx +++ b/packages/react-live/src/components/Live/LivePreview.tsx @@ -18,7 +18,7 @@ function LivePreview({ Component = "div", ...rest }: Props): React.JSX.Element { const { element: Element, onError, newCode } = useContext(LiveContext); return ( - + {Element ? : null} );