diff --git a/.changeset/spotty-melons-listen.md b/.changeset/spotty-melons-listen.md new file mode 100644 index 0000000..36b5c84 --- /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 f08cf16..8b1fca6 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 b949d05..98bf613 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 1533b15..970f516 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} );