[fix] react component runaways, error boundaries (#1625)

This PR fixes a few components that were updating too often. It changes
the format of our error boundaries in order to avoid re-rendering them
as changed props.

### Change Type

- [x] `major` — Breaking change
This commit is contained in:
Steve Ruiz 2023-06-20 15:06:28 +01:00 committed by GitHub
parent 5cb08711c1
commit 83184aaf43
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
11 changed files with 55 additions and 67 deletions

View file

@ -11,7 +11,6 @@ export default function ErrorBoundaryExample() {
shapes={shapes}
tools={[]}
components={{
ErrorFallback: null, // disable app-level error boundaries
ShapeErrorFallback: ({ error }) => <div>Shape error! {String(error)}</div>, // use a custom error fallback for shapes
}}
onMount={(editor) => {

View file

@ -770,7 +770,7 @@ export class ErrorBoundary extends React_3.Component<React_3.PropsWithRef<React_
error: Error;
};
// (undocumented)
render(): React_3.ReactNode;
render(): boolean | JSX.Element | null | number | React_3.ReactFragment | string | undefined;
// (undocumented)
state: TLErrorBoundaryState;
}
@ -1743,7 +1743,7 @@ export function openWindow(url: string, target?: string): void;
// @internal (undocumented)
export function OptionalErrorBoundary({ children, fallback, ...props }: Omit<TLErrorBoundaryProps, 'fallback'> & {
fallback: ((error: unknown) => React_3.ReactNode) | null;
fallback: TLErrorFallbackComponent;
}): JSX.Element;
// @public (undocumented)
@ -2261,7 +2261,7 @@ export interface TLEditorComponents {
// (undocumented)
Cursor: null | TLCursorComponent;
// (undocumented)
ErrorFallback: null | TLErrorFallbackComponent;
ErrorFallback: TLErrorFallbackComponent;
// (undocumented)
Grid: null | TLGridComponent;
// (undocumented)
@ -2269,9 +2269,9 @@ export interface TLEditorComponents {
// (undocumented)
Scribble: null | TLScribbleComponent;
// (undocumented)
ShapeErrorFallback: null | TLShapeErrorFallbackComponent;
ShapeErrorFallback: TLShapeErrorFallbackComponent;
// (undocumented)
ShapeIndicatorErrorFallback: null | TLShapeIndicatorErrorFallback;
ShapeIndicatorErrorFallback: TLShapeIndicatorErrorFallback;
// (undocumented)
SnapLine: null | TLSnapLineComponent;
// (undocumented)
@ -2306,7 +2306,9 @@ export interface TLErrorBoundaryProps {
// (undocumented)
children: React_3.ReactNode;
// (undocumented)
fallback: (error: unknown) => React_3.ReactNode;
fallback: (props: {
error: unknown;
}) => any;
// (undocumented)
onError?: ((error: unknown) => void) | null;
}

View file

@ -126,7 +126,7 @@ export const TldrawEditor = memo(function TldrawEditor({
return (
<div ref={setContainer} draggable={false} className="tl-container tl-theme__light" tabIndex={0}>
<OptionalErrorBoundary
fallback={ErrorFallback ? (error) => <ErrorFallback error={error} /> : null}
fallback={ErrorFallback}
onError={(error) => annotateError(error, { tags: { origin: 'react.tldraw-before-app' } })}
>
{container && (
@ -297,7 +297,7 @@ function TldrawEditorWithReadyStore({
// document in the event of an error to reassure them that their work is
// not lost.
<OptionalErrorBoundary
fallback={ErrorFallback ? (error) => <ErrorFallback error={error} editor={editor} /> : null}
fallback={ErrorFallback}
onError={(error) =>
editor.annotateError(error, { origin: 'react.tldraw', willCrashApp: true })
}

View file

@ -1,7 +1,11 @@
/** @public */
export type TLShapeErrorFallbackComponent = (props: { error: unknown }) => any | null
export type TLShapeErrorFallbackComponent = (props: { error: any }) => any | null
/** @internal */
export const DefaultShapeErrorFallback: TLShapeErrorFallbackComponent = () => {
return <div className="tl-shape-error-boundary" />
export const DefaultShapeErrorFallback: TLShapeErrorFallbackComponent = ({
error,
}: {
error: any
}) => {
return <div className="tl-shape-error-boundary">{error}</div>
}

View file

@ -1,10 +1,11 @@
import * as React from 'react'
import { TLErrorFallbackComponent } from './DefaultErrorFallback'
/** @public */
export interface TLErrorBoundaryProps {
children: React.ReactNode
onError?: ((error: unknown) => void) | null
fallback: (error: unknown) => React.ReactNode
fallback: (props: { error: unknown }) => any
}
type TLErrorBoundaryState = { error: Error | null }
@ -30,7 +31,8 @@ export class ErrorBoundary extends React.Component<
const { error } = this.state
if (error !== null) {
return this.props.fallback(error)
const { fallback: Fallback } = this.props
return <Fallback error={error} />
}
return this.props.children
@ -43,7 +45,7 @@ export function OptionalErrorBoundary({
fallback,
...props
}: Omit<TLErrorBoundaryProps, 'fallback'> & {
fallback: ((error: unknown) => React.ReactNode) | null
fallback: TLErrorFallbackComponent
}) {
if (fallback === null) {
return <>{children}</>

View file

@ -94,6 +94,13 @@ export const Shape = track(function Shape({
const shape = editor.getShapeById(id)
const annotateError = React.useCallback(
(error: any) => {
editor.annotateError(error, { origin: 'react.shape', willCrashApp: false })
},
[editor]
)
if (!shape) return null
const util = editor.getShapeUtil(shape)
@ -108,12 +115,7 @@ export const Shape = track(function Shape({
draggable={false}
>
{!isCulled && (
<OptionalErrorBoundary
fallback={ShapeErrorFallback ? (error) => <ShapeErrorFallback error={error} /> : null}
onError={(error) =>
editor.annotateError(error, { origin: 'react.shape', willCrashApp: false })
}
>
<OptionalErrorBoundary fallback={ShapeErrorFallback} onError={annotateError}>
<InnerShapeBackground shape={shape} util={util} />
</OptionalErrorBoundary>
)}
@ -133,12 +135,7 @@ export const Shape = track(function Shape({
{isCulled && util.canUnmount(shape) ? (
<CulledShape shape={shape} />
) : (
<OptionalErrorBoundary
fallback={ShapeErrorFallback ? (error) => <ShapeErrorFallback error={error} /> : null}
onError={(error) =>
editor.annotateError(error, { origin: 'react.shape', willCrashApp: false })
}
>
<OptionalErrorBoundary fallback={ShapeErrorFallback} onError={annotateError}>
<InnerShape shape={shape} util={util} />
</OptionalErrorBoundary>
)}

View file

@ -31,11 +31,7 @@ export const InnerIndicator = ({ editor, id }: { editor: Editor; id: TLShapeId }
if (!shape.shape) return null
return (
<OptionalErrorBoundary
fallback={
ShapeIndicatorErrorFallback
? (error) => <ShapeIndicatorErrorFallback error={error} />
: null
}
fallback={ShapeIndicatorErrorFallback}
onError={(error) =>
editor.annotateError(error, { origin: 'react.shapeIndicator', willCrashApp: false })
}

View file

@ -39,9 +39,9 @@ export interface TLEditorComponents {
CollaboratorScribble: TLScribbleComponent | null
SnapLine: TLSnapLineComponent | null
Handle: TLHandleComponent | null
ErrorFallback: TLErrorFallbackComponent | null
ShapeErrorFallback: TLShapeErrorFallbackComponent | null
ShapeIndicatorErrorFallback: TLShapeIndicatorErrorFallbackComponent | null
ErrorFallback: TLErrorFallbackComponent
ShapeErrorFallback: TLShapeErrorFallbackComponent
ShapeIndicatorErrorFallback: TLShapeIndicatorErrorFallbackComponent
Spinner: TLSpinnerComponent | null
}

View file

@ -38,7 +38,7 @@ function checkAllShapes(editor: Editor, shapes: string[]) {
describe('<TldrawEditor />', () => {
it('Renders without crashing', async () => {
render(
<TldrawEditor autoFocus components={{ ErrorFallback: null }}>
<TldrawEditor autoFocus>
<div data-testid="canvas-1" />
</TldrawEditor>
)
@ -49,7 +49,6 @@ describe('<TldrawEditor />', () => {
let editor: Editor
render(
<TldrawEditor
components={{ ErrorFallback: null }}
onMount={(e) => {
editor = e
}}
@ -66,7 +65,6 @@ describe('<TldrawEditor />', () => {
let editor: Editor
render(
<TldrawEditor
components={{ ErrorFallback: null }}
shapes={defaultShapes}
onMount={(e) => {
editor = e
@ -100,7 +98,6 @@ describe('<TldrawEditor />', () => {
const store = createTLStore({ shapes: [] })
render(
<TldrawEditor
components={{ ErrorFallback: null }}
store={store}
onMount={(editor) => {
expect(editor.store).toBe(store)
@ -119,9 +116,13 @@ describe('<TldrawEditor />', () => {
render(
<TldrawEditor
shapes={defaultShapes}
components={{ ErrorFallback: null }}
store={createTLStore({ shapes: [] })}
autoFocus
components={{
ErrorFallback: ({ error }) => {
throw error
},
}}
>
<div data-testid="canvas-1" />
</TldrawEditor>
@ -133,9 +134,13 @@ describe('<TldrawEditor />', () => {
expect(() =>
render(
<TldrawEditor
components={{ ErrorFallback: null }}
store={createTLStore({ shapes: defaultShapes })}
autoFocus
components={{
ErrorFallback: ({ error }) => {
throw error
},
}}
>
<div data-testid="canvas-1" />
</TldrawEditor>
@ -150,12 +155,7 @@ describe('<TldrawEditor />', () => {
const initialStore = createTLStore({ shapes: [] })
const onMount = jest.fn()
const rendered = render(
<TldrawEditor
components={{ ErrorFallback: null }}
store={initialStore}
onMount={onMount}
autoFocus
>
<TldrawEditor store={initialStore} onMount={onMount} autoFocus>
<div data-testid="canvas-1" />
</TldrawEditor>
)
@ -165,12 +165,7 @@ describe('<TldrawEditor />', () => {
expect(initialEditor.store).toBe(initialStore)
// re-render with the same store:
rendered.rerender(
<TldrawEditor
components={{ ErrorFallback: null }}
store={initialStore}
onMount={onMount}
autoFocus
>
<TldrawEditor store={initialStore} onMount={onMount} autoFocus>
<div data-testid="canvas-2" />
</TldrawEditor>
)
@ -180,12 +175,7 @@ describe('<TldrawEditor />', () => {
// re-render with a new store:
const newStore = createTLStore({ shapes: [] })
rendered.rerender(
<TldrawEditor
components={{ ErrorFallback: null }}
store={newStore}
onMount={onMount}
autoFocus
>
<TldrawEditor store={newStore} onMount={onMount} autoFocus>
<div data-testid="canvas-3" />
</TldrawEditor>
)
@ -199,7 +189,6 @@ describe('<TldrawEditor />', () => {
let editor = {} as Editor
render(
<TldrawEditor
components={{ ErrorFallback: null }}
shapes={defaultShapes}
tools={defaultTools}
autoFocus
@ -322,7 +311,6 @@ describe('Custom shapes', () => {
let editor = {} as Editor
render(
<TldrawEditor
components={{ ErrorFallback: null }}
shapes={shapes}
tools={tools}
autoFocus

View file

@ -1,7 +1,7 @@
import { GeoShapeGeoStyle, preventDefault, useEditor } from '@tldraw/editor'
import { track, useValue } from '@tldraw/state'
import classNames from 'classnames'
import React from 'react'
import React, { memo } from 'react'
import { useBreakpoint } from '../../hooks/useBreakpoint'
import { useReadonly } from '../../hooks/useReadonly'
import { TLUiToolbarItem, useToolbarSchema } from '../../hooks/useToolbarSchema'
@ -19,7 +19,7 @@ import { kbdStr } from '../primitives/shared'
import { ToggleToolLockedButton } from './ToggleToolLockedButton'
/** @public */
export const Toolbar = function Toolbar() {
export const Toolbar = memo(function Toolbar() {
const editor = useEditor()
const msg = useTranslation()
const breakpoint = useBreakpoint()
@ -217,7 +217,7 @@ export const Toolbar = function Toolbar() {
</div>
</div>
)
}
})
const OverflowToolsContent = track(function OverflowToolsContent({
toolbarItems,

View file

@ -1,6 +1,6 @@
import { Range, Root, Thumb, Track } from '@radix-ui/react-slider'
import { useEditor } from '@tldraw/editor'
import { useCallback } from 'react'
import { memo, useCallback } from 'react'
import { TLUiTranslationKey } from '../../hooks/useTranslation/TLUiTranslationKey'
import { useTranslation } from '../../hooks/useTranslation/useTranslation'
@ -15,7 +15,7 @@ export interface SliderProps {
}
/** @internal */
export function Slider(props: SliderProps) {
export const Slider = memo(function Slider(props: SliderProps) {
const { title, steps, value, label, onValueChange } = props
const editor = useEditor()
const msg = useTranslation()
@ -59,4 +59,4 @@ export function Slider(props: SliderProps) {
</Root>
</div>
)
}
})