Skip to content

Commit 49da66a

Browse files
authored
fix(web): isDestroyed editor check (#777)
# Summary There is a well-known issue with TipTap editor where it doesn't necessarily check for its `isDestroyed` state, when trying to use e.g. `commands`. ueberdosis/tiptap#1451 Here this flow resulted in an uncaught error `Cannot read properties of null (reading 'commands')`: There was a found edge-case, when you re-rendered `EnrichedTextInput` with new `htmlStyle` and `defaultValue`: 1. `resolvedHtml` changes because of the new `htmlStyle` 2. `tiptapContent` changes because of the new `defaultValue` 3. `useEditor` runs and recreates the input as `tiptapContent` changed - this is a hook so actual recreation will happen after the render 4. `useEffect` with `commands.normalizeBoldInStyledHeadings` gets scheduled to run after the render. The closure here captures the **stale** `editor`, before `useEditor` actually ran. 5. After render `useEditor` successfully runs and updates `editor` 6. The latter `useEffect` runs with a `stale` editor, which crashes the app After the added check the `6.` will never run, but `editor` changed, so the same `useEffect` will correctly run on the next render. Provided safety-checks, when using `commands`, outside of the TipTap internal state, in `EnrichedTextInput`. all web e2e tests pass ## Compatibility | OS | Implemented | | ------- | :---------: | | iOS | ❌ | | Android | ❌ | | Web | ✅ | ## Checklist - [x] E2E tests are passing - [ ] Required E2E tests have been added (N/A)
1 parent ab45e3d commit 49da66a

6 files changed

Lines changed: 193 additions & 19 deletions

File tree

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,45 @@
1+
import { test, expect } from '@playwright/test';
2+
3+
const ROOT_SELECTOR = '[data-testid="test-render-cycle-root"]';
4+
const EDITOR_SELECTOR = `${ROOT_SELECTOR} .eti-editor [contenteditable="true"]`;
5+
const TOGGLE_BUTTON_SELECTOR = '[data-testid="toggle-variant-button"]';
6+
const VARIANT_OUTPUT_SELECTOR = '[data-testid="variant-output"]';
7+
const PAGE_PATH = '/test-render-cycle';
8+
9+
test.describe('EnrichedTextInput render cycle', () => {
10+
test('does not throw on simultaneous defaultValue and htmlStyle change', async ({
11+
page,
12+
}) => {
13+
const pageErrors: Error[] = [];
14+
const consoleErrors: string[] = [];
15+
16+
page.on('pageerror', (error) => pageErrors.push(error));
17+
page.on('console', (message) => {
18+
if (message.type() === 'error') {
19+
consoleErrors.push(message.text());
20+
}
21+
});
22+
23+
await page.goto(PAGE_PATH);
24+
await page.waitForSelector(EDITOR_SELECTOR);
25+
26+
await expect(page.locator(EDITOR_SELECTOR)).toContainText('Variant A');
27+
28+
await page.click(TOGGLE_BUTTON_SELECTOR);
29+
await expect(page.locator(VARIANT_OUTPUT_SELECTOR)).toHaveText('b');
30+
await expect(page.locator(EDITOR_SELECTOR)).toContainText('Variant B');
31+
32+
await page.click(TOGGLE_BUTTON_SELECTOR);
33+
await expect(page.locator(VARIANT_OUTPUT_SELECTOR)).toHaveText('a');
34+
await expect(page.locator(EDITOR_SELECTOR)).toContainText('Variant A');
35+
36+
const editor = page.locator(EDITOR_SELECTOR);
37+
await editor.click();
38+
await expect(editor).toBeFocused();
39+
await editor.pressSequentially(' more text');
40+
await expect(editor).toContainText('Variant A more text');
41+
42+
expect(pageErrors).toEqual([]);
43+
expect(consoleErrors).toEqual([]);
44+
});
45+
});

apps/example-web/src/RouteSelector.tsx

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import { VisualRegression } from './testScreens/VisualRegression';
66
import { TestSubmitProps } from './testScreens/TestSubmitProps';
77
import { TestEnrichedText } from './testScreens/TestEnrichedText';
88
import { TestEllipsize } from './testScreens/TestEllipsize';
9+
import { TestRenderCycle } from './testScreens/TestRenderCycle';
910
import { useEffect, useState } from 'react';
1011

1112
export default function RouteSelector() {
@@ -50,5 +51,9 @@ export default function RouteSelector() {
5051
return <TestEllipsize />;
5152
}
5253

54+
if (path === '/test-render-cycle') {
55+
return <TestRenderCycle />;
56+
}
57+
5358
return <App />;
5459
}
Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import { useMemo, useRef, useState } from 'react';
2+
import {
3+
EnrichedTextInput,
4+
type EnrichedTextInputInstance,
5+
type HtmlStyle,
6+
} from 'react-native-enriched-html';
7+
import { WEB_DEFAULT_HTML_STYLE } from '../defaultHtmlStyle';
8+
9+
const VARIANTS = {
10+
a: {
11+
defaultValue: '<p>Variant A</p>',
12+
htmlStyle: WEB_DEFAULT_HTML_STYLE,
13+
},
14+
b: {
15+
defaultValue: '<h1>Variant B</h1>',
16+
htmlStyle: { ...WEB_DEFAULT_HTML_STYLE, h1: { fontSize: 48 } },
17+
},
18+
} as const satisfies Record<
19+
string,
20+
{ defaultValue: string; htmlStyle: HtmlStyle }
21+
>;
22+
23+
export function TestRenderCycle() {
24+
const ref = useRef<EnrichedTextInputInstance>(null);
25+
const [variant, setVariant] = useState<keyof typeof VARIANTS>('a');
26+
27+
const { defaultValue, htmlStyle } = useMemo(
28+
() => VARIANTS[variant],
29+
[variant]
30+
);
31+
32+
return (
33+
<div data-testid="test-render-cycle-root">
34+
<div
35+
className="editor-wrapper"
36+
style={editorContainerStyle}
37+
data-testid="editor-container"
38+
onClick={() => ref.current?.focus()}
39+
>
40+
<EnrichedTextInput
41+
ref={ref}
42+
defaultValue={defaultValue}
43+
htmlStyle={htmlStyle}
44+
placeholder="Test editor"
45+
autoFocus
46+
editable
47+
scrollEnabled
48+
/>
49+
</div>
50+
51+
<button
52+
type="button"
53+
data-testid="toggle-variant-button"
54+
onClick={() => {
55+
setVariant((prev) => (prev === 'a' ? 'b' : 'a'));
56+
}}
57+
>
58+
Toggle variant
59+
</button>
60+
61+
<pre data-testid="variant-output">{variant}</pre>
62+
</div>
63+
);
64+
}
65+
66+
const editorContainerStyle = {
67+
backgroundColor: '#ddd',
68+
padding: '16px',
69+
borderRadius: '8px',
70+
} as const;

src/web/EnrichedTextInput.tsx

Lines changed: 25 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -87,12 +87,13 @@ import {
8787
sanitizeMentionAttributes,
8888
} from './sanitization/htmlSanitizer';
8989
import { assertBrowserEnvironment } from './utils/assertBrowserEnvironment';
90+
import { runSafelyInEditor } from './utils/runSafelyInEditor';
9091

9192
function runFocused(
9293
editor: Editor,
9394
apply: (chain: ChainedCommands) => ChainedCommands
9495
) {
95-
apply(editor.chain().focus()).run();
96+
runSafelyInEditor(editor, (e) => apply(e.chain().focus()).run());
9697
}
9798

9899
export const EnrichedTextInput = ({
@@ -180,7 +181,7 @@ export const EnrichedTextInput = ({
180181
const text = nativeLeafText(doc, 0, doc.content.size);
181182
onSubmitEditingRef.current?.(adaptWebToNativeEvent(event, { text }));
182183
if (sb === 'blurAndSubmit') {
183-
editorInstanceRef.current?.commands.blur();
184+
runSafelyInEditor(editorInstanceRef.current, (e) => e.commands.blur());
184185
}
185186
return true;
186187
}
@@ -259,7 +260,9 @@ export const EnrichedTextInput = ({
259260
autofocus: autoFocus,
260261
onCreate: ({ editor: _editor }) => {
261262
// Setting initial content in this way ensures all custom plugins are run and applied
262-
_editor.commands.setContent(tiptapContent ?? '');
263+
runSafelyInEditor(_editor, (e) =>
264+
e.commands.setContent(tiptapContent ?? '')
265+
);
263266
},
264267
onFocus: ({ event }) => {
265268
onFocus?.(adaptWebToNativeEvent(event, { target: -1 }));
@@ -319,7 +322,9 @@ export const EnrichedTextInput = ({
319322
}, [editor, returnKeyType]);
320323

321324
useEffect(() => {
322-
editor?.commands.normalizeBoldInStyledHeadings();
325+
runSafelyInEditor(editor, (e) =>
326+
e.commands.normalizeBoldInStyledHeadings()
327+
);
323328
}, [editor, resolvedHtmlStyle]);
324329

325330
const getMentionCallbacks = useCallback(
@@ -336,14 +341,16 @@ export const EnrichedTextInput = ({
336341
useImperativeHandle(
337342
ref,
338343
(): EnrichedTextInputInstance => ({
339-
focus: () => editor.commands.focus(),
340-
blur: () => editor.commands.blur(),
344+
focus: () => runSafelyInEditor(editor, (e) => e.commands.focus()),
345+
blur: () => runSafelyInEditor(editor, (e) => e.commands.blur()),
341346
setValue: (value: string) =>
342-
editor.commands.setContent(
343-
prepareHtmlForTiptap(
344-
value,
345-
useHtmlNormalizerRef.current,
346-
sanitizationConfigRef.current
347+
runSafelyInEditor(editor, (e) =>
348+
e.commands.setContent(
349+
prepareHtmlForTiptap(
350+
value,
351+
useHtmlNormalizerRef.current,
352+
sanitizationConfigRef.current
353+
)
347354
)
348355
),
349356
setSelection: (start, end) => {
@@ -381,23 +388,22 @@ export const EnrichedTextInput = ({
381388
toggleCheckboxList: (checked: boolean) =>
382389
runFocused(editor, (c) => c.toggleCheckboxList(checked)),
383390
setLink: (start: number, end: number, text: string, url: string) =>
384-
setLink(editor, start, end, text, url),
391+
runSafelyInEditor(editor, (e) => setLink(e, start, end, text, url)),
385392
removeLink: (start: number, end: number) =>
386-
removeLink(editor, start, end),
393+
runSafelyInEditor(editor, (e) => removeLink(e, start, end)),
387394
startMention: (indicator: string) => {
388-
startMention(editor, indicator, mentionIndicatorsRef.current);
395+
runSafelyInEditor(editor, (e) =>
396+
startMention(e, indicator, mentionIndicatorsRef.current)
397+
);
389398
},
390399
setMention: (
391400
indicator: string,
392401
text: string,
393402
attributes?: Record<string, string>
394403
) => {
395404
checkMentionAttributes(attributes);
396-
setMention(
397-
editor,
398-
indicator,
399-
text,
400-
sanitizeMentionAttributes(attributes)
405+
runSafelyInEditor(editor, (e) =>
406+
setMention(e, indicator, text, sanitizeMentionAttributes(attributes))
401407
);
402408
},
403409
setImage: (src: string, width: number, height: number) =>
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
import type { Editor } from '@tiptap/react';
2+
import { runSafelyInEditor } from '../utils/runSafelyInEditor';
3+
4+
function makeEditor(isDestroyed: boolean): Editor {
5+
return { isDestroyed } as Editor;
6+
}
7+
8+
describe('runSafelyInEditor', () => {
9+
test('runs the callback and returns its result when editor is alive', () => {
10+
const editor = makeEditor(false);
11+
const callback = jest.fn((e: Editor) => e);
12+
13+
const result = runSafelyInEditor(editor, callback);
14+
15+
expect(callback).toHaveBeenCalledWith(editor);
16+
expect(result).toBe(editor);
17+
});
18+
19+
test('does not run the callback and returns null when editor is destroyed', () => {
20+
const editor = makeEditor(true);
21+
const callback = jest.fn();
22+
23+
const result = runSafelyInEditor(editor, callback);
24+
25+
expect(callback).not.toHaveBeenCalled();
26+
expect(result).toBeNull();
27+
});
28+
29+
test('does not run the callback and returns null when editor is null', () => {
30+
const callback = jest.fn();
31+
32+
const result = runSafelyInEditor(null, callback);
33+
34+
expect(callback).not.toHaveBeenCalled();
35+
expect(result).toBeNull();
36+
});
37+
});

src/web/utils/runSafelyInEditor.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
import type { Editor } from '@tiptap/react';
2+
3+
export function runSafelyInEditor<T>(
4+
editor: Editor | null,
5+
callback: (editor: Editor) => T
6+
): T | null {
7+
if (editor && !editor.isDestroyed) {
8+
return callback(editor);
9+
}
10+
return null;
11+
}

0 commit comments

Comments
 (0)