Fix multiple editor instances issue (#4001)
React's strict mode runs effects twice on mount, but once it's done that it'll go forward with the state from the first effect. For example, this component: ```tsx let nextId = 1 function Component() { const [state, setState] = useState(null) useEffect(() => { const id = nextId++ console.log('set up', id) setState(id) return () => console.log('tear down', id) }, []) if (!state) return console.log('render', state) } ``` Would log something like this when mounting for the first time: - `set up 1` - `tear down 1` - `set up 2` - `render 1` For us, this is a problem: editor 2 is the version that's still running, but editor 1 is getting used for render. React talks a bit about this issue here: https://github.com/reactwg/react-18/discussions/19 The fix seems to be to keep the editor in a `useRef` instead of a `useState`. We need the state to trigger re-renders though, so we sync the ref into the state although we don't actually use the state value. ### Change Type - [x] `sdk` — Changes the tldraw SDK - [x] `bugfix` — Bug fix ### Release Notes - Fix a bug causing text shape measurement to work incorrectly when using react strict mode
This commit is contained in:
parent
480c23135f
commit
2d2a7ea76f
4 changed files with 37 additions and 9 deletions
|
@ -12,8 +12,6 @@ import { examples } from './examples'
|
||||||
import Develop from './misc/develop'
|
import Develop from './misc/develop'
|
||||||
import EndToEnd from './misc/end-to-end'
|
import EndToEnd from './misc/end-to-end'
|
||||||
|
|
||||||
// This example is only used for end to end tests
|
|
||||||
|
|
||||||
// we use secret internal `setDefaultAssetUrls` functions to set these at the
|
// we use secret internal `setDefaultAssetUrls` functions to set these at the
|
||||||
// top-level so assets don't need to be passed down in every single example.
|
// top-level so assets don't need to be passed down in every single example.
|
||||||
const assetUrls = getAssetUrlsByMetaUrl()
|
const assetUrls = getAssetUrlsByMetaUrl()
|
||||||
|
|
|
@ -7,6 +7,7 @@ import React, {
|
||||||
useCallback,
|
useCallback,
|
||||||
useLayoutEffect,
|
useLayoutEffect,
|
||||||
useMemo,
|
useMemo,
|
||||||
|
useRef,
|
||||||
useState,
|
useState,
|
||||||
useSyncExternalStore,
|
useSyncExternalStore,
|
||||||
} from 'react'
|
} from 'react'
|
||||||
|
@ -316,7 +317,17 @@ function TldrawEditorWithReadyStore({
|
||||||
>) {
|
>) {
|
||||||
const { ErrorFallback } = useEditorComponents()
|
const { ErrorFallback } = useEditorComponents()
|
||||||
const container = useContainer()
|
const container = useContainer()
|
||||||
const [editor, setEditor] = useState<Editor | null>(null)
|
const editorRef = useRef<Editor | null>(null)
|
||||||
|
// we need to store the editor instance in a ref so that it persists across strict-mode
|
||||||
|
// remounts, but that won't trigger re-renders, so we use this hook to make sure all child
|
||||||
|
// components get the most up to date editor reference when needed.
|
||||||
|
const [renderEditor, setRenderEditor] = useState<Editor | null>(null)
|
||||||
|
|
||||||
|
const editor = editorRef.current
|
||||||
|
if (renderEditor !== editor) {
|
||||||
|
setRenderEditor(editor)
|
||||||
|
}
|
||||||
|
|
||||||
const [initialAutoFocus] = useState(autoFocus)
|
const [initialAutoFocus] = useState(autoFocus)
|
||||||
|
|
||||||
useLayoutEffect(() => {
|
useLayoutEffect(() => {
|
||||||
|
@ -334,7 +345,9 @@ function TldrawEditorWithReadyStore({
|
||||||
assetOptions,
|
assetOptions,
|
||||||
options,
|
options,
|
||||||
})
|
})
|
||||||
setEditor(editor)
|
|
||||||
|
editorRef.current = editor
|
||||||
|
setRenderEditor(editor)
|
||||||
|
|
||||||
return () => {
|
return () => {
|
||||||
editor.dispose()
|
editor.dispose()
|
||||||
|
|
|
@ -43,18 +43,16 @@ export class TextManager {
|
||||||
constructor(public editor: Editor) {
|
constructor(public editor: Editor) {
|
||||||
const container = this.editor.getContainer()
|
const container = this.editor.getContainer()
|
||||||
|
|
||||||
// Remove any existing text measure element that
|
|
||||||
// is a descendant of this editor's container
|
|
||||||
container.querySelector('#tldraw_text_measure')?.remove()
|
|
||||||
|
|
||||||
const elm = document.createElement('div')
|
const elm = document.createElement('div')
|
||||||
elm.id = `tldraw_text_measure`
|
|
||||||
elm.classList.add('tl-text')
|
elm.classList.add('tl-text')
|
||||||
elm.classList.add('tl-text-measure')
|
elm.classList.add('tl-text-measure')
|
||||||
elm.tabIndex = -1
|
elm.tabIndex = -1
|
||||||
container.appendChild(elm)
|
container.appendChild(elm)
|
||||||
|
|
||||||
this.baseElm = elm
|
this.baseElm = elm
|
||||||
|
editor.disposables.add(() => {
|
||||||
|
elm.remove()
|
||||||
|
})
|
||||||
}
|
}
|
||||||
|
|
||||||
measureText = (
|
measureText = (
|
||||||
|
|
|
@ -10,6 +10,7 @@ import {
|
||||||
createTLStore,
|
createTLStore,
|
||||||
noop,
|
noop,
|
||||||
} from '@tldraw/editor'
|
} from '@tldraw/editor'
|
||||||
|
import { StrictMode } from 'react'
|
||||||
import { defaultTools } from '../lib/defaultTools'
|
import { defaultTools } from '../lib/defaultTools'
|
||||||
import { GeoShapeUtil } from '../lib/shapes/geo/GeoShapeUtil'
|
import { GeoShapeUtil } from '../lib/shapes/geo/GeoShapeUtil'
|
||||||
import { renderTldrawComponent } from './testutils/renderTldrawComponent'
|
import { renderTldrawComponent } from './testutils/renderTldrawComponent'
|
||||||
|
@ -207,6 +208,24 @@ describe('<TldrawEditor />', () => {
|
||||||
// Is the editor's current tool correct?
|
// Is the editor's current tool correct?
|
||||||
expect(editor.getCurrentToolId()).toBe('eraser')
|
expect(editor.getCurrentToolId()).toBe('eraser')
|
||||||
})
|
})
|
||||||
|
|
||||||
|
it('renders correctly in strict mode', async () => {
|
||||||
|
const editorInstances = new Set<Editor>()
|
||||||
|
const onMount = jest.fn((editor: Editor) => {
|
||||||
|
editorInstances.add(editor)
|
||||||
|
})
|
||||||
|
await renderTldrawComponent(
|
||||||
|
<StrictMode>
|
||||||
|
<TldrawEditor tools={defaultTools} initialState="select" onMount={onMount} />
|
||||||
|
</StrictMode>,
|
||||||
|
{ waitForPatterns: false }
|
||||||
|
)
|
||||||
|
|
||||||
|
// we should only get one editor instance
|
||||||
|
expect(editorInstances.size).toBe(1)
|
||||||
|
// but strict mode will cause onMount to be called twice
|
||||||
|
expect(onMount).toHaveBeenCalledTimes(2)
|
||||||
|
})
|
||||||
})
|
})
|
||||||
|
|
||||||
describe('Custom shapes', () => {
|
describe('Custom shapes', () => {
|
||||||
|
|
Loading…
Add table
Add a link
Reference in a new issue