kokhlo opened a new pull request, #44373:
URL: https://github.com/apache/superset/pull/44373

   ### SUMMARY
   Fixes #44371
   
   Typing with several cursors in SQL Lab collapses the multi-selection 
mid-keystroke and moves a cursor to the document end, so subsequent characters 
land at the wrong position.
   
   Root cause: with N cursors Ace applies a keystroke per selection 
(`forEachSelection`), so the session `change` event fires N times with N 
intermediate document values, and every firing flows through `onChange` into 
the controlled `value` prop (`EditorWrapper` → `EditorHost` → 
`AceEditorProvider` → react-ace). When React renders with an intermediate value 
while the editor already holds the final one, react-ace's `componentDidUpdate` 
sees `getValue() !== props.value` and calls `editor.setValue()` mid-keystroke — 
on 6.0.0 that unconditionally resets selection to the document end 
(`navigateFileEnd`, exactly the reporter's trace); on current master the 
save/restore around it uses `selection.toJSON()/fromJSON()`, which is a 
single-range API, so a multi-selection still collapses to its first range. 
Either way the extra cursors are destroyed while typing continues.
   
   Fix: in `AceEditorProvider`, coalesce change emissions per task — buffer the 
latest editor value and invoke the consumer `onChange` once per microtask. 
Intermediate per-selection values never reach the controlled `value` prop, 
`getValue() === props.value` holds at every render, and react-ace's out-of-band 
`setValue` never fires while typing. A pending value is still delivered if the 
editor unmounts mid-keystroke (tab switch). Programmatic value changes (format 
SQL etc.) keep the existing single-range restore path, which is correct for 
single-cursor restores.
   
   Placed in the provider so every editor surface (SQL Lab, template params, 
dashboard editors) benefits.
   
   ### TESTING
   - `npx jest src/core/editors/AceEditorProvider.test.tsx` — 6 passed, 
including two new regression tests: coalescing (asserts a single `onChange` 
emission with the final value after two synchronous editor change events — 
fails on current master) and unmount delivery.
   - `npx jest src/core/editors/EditorHost.test.tsx 
src/SqlLab/components/EditorWrapper/EditorWrapper.test.tsx 
src/SqlLab/components/TemplateParamsEditor 
src/components/SQLEditorWithValidation` — 30 passed, no regressions on the 
consumer suites.
   - RED verified by sabotage: reverting the provider to pass the raw 
`onChange` through makes the new coalescing test fail, restoring the fix makes 
it pass.
   - `npx oxlint --config oxlint.json` on touched files: no new findings vs 
master (4 pre-existing warnings, identical on pristine master).
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to