fix(site-builder): stop CodeEditor fallback insert from being lost on CodeMirror mount
insertAtCursor's textarea-fallback branch advanced lastEmittedRef to the post-insertion value. The mount effect's dynamic import() closes over `value` as of initial render, so if CodeMirror finishes loading after a fallback-mode insertion, it mounts with the pre-insertion doc. The only repair mechanism -- the value-sync effect -- is gated on `value !== lastEmittedRef.current`, so advancing that ref made the gate see them as already equal and skip the repair, silently dropping the insertion. Also clamps caretOffset to [0, text.length] defensively, and adds coverage for a non-empty-selection replace and for the fallback-to-CodeMirror-mount transition itself (using the real @codemirror/* packages, no mocks). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
@@ -122,4 +122,72 @@ describe('CodeEditor imperative handle (textarea fallback mode)', () => {
|
||||
});
|
||||
expect(ta.selectionStart).toBe(3);
|
||||
});
|
||||
|
||||
test('insertAtCursor replaces a non-empty selection (not just an empty caret)', () => {
|
||||
const onChange = vi.fn();
|
||||
const ref = React.createRef<CodeEditorHandle>();
|
||||
render(<CodeEditor ref={ref} value="<div>old</div>" onChange={onChange} />);
|
||||
|
||||
const ta = container.querySelector('[data-testid="code-editor-fallback"]') as HTMLTextAreaElement;
|
||||
// "<div>old</div>" -- select "old" (indices 5-8).
|
||||
ta.selectionStart = 5;
|
||||
ta.selectionEnd = 8;
|
||||
|
||||
act(() => {
|
||||
ref.current!.insertAtCursor('new');
|
||||
});
|
||||
|
||||
expect(onChange).toHaveBeenCalledWith('<div>new</div>');
|
||||
});
|
||||
});
|
||||
|
||||
describe('CodeEditor imperative handle: fallback insert survives a later CodeMirror mount', () => {
|
||||
// Regression test for a data-loss bug: insertAtCursor's textarea-fallback
|
||||
// branch used to advance lastEmittedRef to the post-insertion value. The
|
||||
// mount effect's dynamic import() closes over `value` as of the initial
|
||||
// render, so if CodeMirror finishes loading *after* a fallback-mode
|
||||
// insertion, it mounts with the pre-insertion doc. The only thing that
|
||||
// repairs that is the value-sync effect, which is gated on `value !==
|
||||
// lastEmittedRef.current` -- advancing lastEmittedRef made that gate see
|
||||
// them as already equal and skip the repair, silently dropping the
|
||||
// insertion. This test drives the component through that exact sequence
|
||||
// using the real @codemirror/* packages (no mocks, no fake timers) to
|
||||
// prove the fix holds.
|
||||
test('insertAtCursor in fallback mode is not lost once CodeMirror mounts', async () => {
|
||||
const handleRef = React.createRef<CodeEditorHandle>();
|
||||
function Harness() {
|
||||
const [value, setValue] = React.useState('<div></div>');
|
||||
return <CodeEditor ref={handleRef} value={value} onChange={setValue} />;
|
||||
}
|
||||
render(<Harness />);
|
||||
|
||||
// First tick: the dynamic import() chain is always async, so this is
|
||||
// still the textarea fallback (see the file-level comment above).
|
||||
const ta = container.querySelector('[data-testid="code-editor-fallback"]') as HTMLTextAreaElement;
|
||||
expect(ta).not.toBeNull();
|
||||
ta.selectionStart = 5;
|
||||
ta.selectionEnd = 5;
|
||||
act(() => {
|
||||
handleRef.current!.insertAtCursor('<p></p>');
|
||||
});
|
||||
|
||||
// Let the real dynamic import() of @codemirror/* actually resolve and
|
||||
// the view mount (real elapsed time, not mocked/faked).
|
||||
for (let i = 0; i < 5; i += 1) {
|
||||
// eslint-disable-next-line no-await-in-loop
|
||||
await act(async () => {
|
||||
await new Promise((resolve) => { setTimeout(resolve, 50); });
|
||||
});
|
||||
}
|
||||
|
||||
// Confirm CodeMirror actually mounted (fallback textarea gone, replaced
|
||||
// by the CodeMirror root) -- otherwise this assertion would trivially
|
||||
// pass by reading back the fallback textarea's own value and wouldn't
|
||||
// exercise the bug at all.
|
||||
expect(container.querySelector('[data-testid="code-editor-fallback"]')).toBeNull();
|
||||
|
||||
// The value-sync effect must have pushed the post-insertion value into
|
||||
// the freshly-mounted doc -- the insertion must not have been dropped.
|
||||
expect(handleRef.current!.getValue()).toBe('<div><p></p></div>');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -109,6 +109,15 @@ function loadCodeMirror(): Promise<CmModules> {
|
||||
return cmModulesPromise;
|
||||
}
|
||||
|
||||
// Guards against a caller passing a `caretOffset` outside [0, text.length]
|
||||
// (e.g. a stale offset computed against different snippet text), which
|
||||
// would otherwise let `insertAtCursor` compute a caret position past the
|
||||
// text it just inserted.
|
||||
function clampCaretOffset(caretOffset: number | undefined, textLength: number): number {
|
||||
if (caretOffset === undefined) return textLength;
|
||||
return Math.min(Math.max(caretOffset, 0), textLength);
|
||||
}
|
||||
|
||||
function languageExtension(mods: CmModules, language: CodeEditorLanguage) {
|
||||
switch (language) {
|
||||
case 'css':
|
||||
@@ -246,7 +255,7 @@ export const CodeEditor = forwardRef<CodeEditorHandle, CodeEditorProps>(function
|
||||
const view = viewRef.current;
|
||||
if (view) {
|
||||
const { from, to } = view.state.selection.main;
|
||||
const caret = from + (caretOffset ?? text.length);
|
||||
const caret = from + clampCaretOffset(caretOffset, text.length);
|
||||
view.dispatch({
|
||||
changes: { from, to, insert: text },
|
||||
selection: { anchor: caret },
|
||||
@@ -254,15 +263,25 @@ export const CodeEditor = forwardRef<CodeEditorHandle, CodeEditorProps>(function
|
||||
view.focus();
|
||||
return;
|
||||
}
|
||||
// Textarea fallback: CodeMirror never mounted (still loading, or its
|
||||
// lazy chunks failed). The toolbar must keep working either way.
|
||||
// Textarea fallback: CodeMirror never mounted yet (still loading, or
|
||||
// its lazy chunks failed). The toolbar must keep working either way.
|
||||
//
|
||||
// Deliberately do NOT touch lastEmittedRef here -- same as the plain
|
||||
// textarea onChange handler below, which never touches it either.
|
||||
// The mount effect's dynamic import() closes over `value` at the time
|
||||
// it started, so CodeMirror can finish loading with a stale doc if it
|
||||
// resolves after this insertion. The only thing that catches that is
|
||||
// the value-sync effect above, which is gated on `value !==
|
||||
// lastEmittedRef.current`. If this insertion advanced lastEmittedRef
|
||||
// to `next`, that effect would see value === lastEmittedRef.current
|
||||
// once the parent re-renders and silently skip pushing the insertion
|
||||
// into the freshly-mounted (stale) doc -- losing it for good.
|
||||
const ta = textareaRef.current;
|
||||
if (!ta) return;
|
||||
const from = ta.selectionStart ?? ta.value.length;
|
||||
const to = ta.selectionEnd ?? from;
|
||||
const next = ta.value.slice(0, from) + text + ta.value.slice(to);
|
||||
const caret = from + (caretOffset ?? text.length);
|
||||
lastEmittedRef.current = next;
|
||||
const caret = from + clampCaretOffset(caretOffset, text.length);
|
||||
onChangeRef.current(next);
|
||||
ta.value = next;
|
||||
ta.selectionStart = caret;
|
||||
|
||||
Reference in New Issue
Block a user