snuba icon indicating copy to clipboard operation
snuba copied to clipboard

Snuba admin breaks browser undo

Open onewland opened this issue 1 year ago • 3 comments

If you delete a string in the System Queries tool (probably other SQL text entries as well) and then Control+ or Command+Z in the browser, the undo doesn't perform any action. We should not override browser behavior if we're not doing anything else useful.

onewland avatar Jan 16 '25 19:01 onewland

the answer for this is probably to use basicSetup to give codemirror reasonable defaults: https://codemirror.net/docs/ref/#codemirror.basicSetup

currently: https://github.com/getsentry/snuba/blob/master/snuba/admin/static/common/components/sql_editor.tsx#L31-L37

just want to check with @gggritso that there's nothing special in our logic here

onewland avatar Feb 03 '25 23:02 onewland

basicSetup and minimalSetup seem to do something that conflicts with the DOM so maybe we just add in a custom history handler and call it a day:

hook.js:608  Warning: validateDOMNesting(...): <form> cannot appear as a descendant of <form>. Error Component Stack
    at form (<anonymous>)
    at QueryEditor (query_editor.tsx:51:29)
    at form (<anonymous>)
    at div (<anonymous>)
    at div (<anonymous>)
    at ClickhouseQueries (index.tsx:8:63)
    at div (<anonymous>)
    at Body (body.tsx:12:11)
    at div (<anonymous>)
    at div (<anonymous>)
Uncaught Error: Unrecognized extension value in extension set ([object Object]). This sometimes happens because multiple instances of @codemirror/state are loaded, breaking instanceof checks.
    at inner (index.js:2044:23)
    at inner (index.js:2019:17)
    at inner (index.js:2019:17)
    at inner (index.js:2019:17)
    at flatten (index.js:2048:5)
    at _Configuration.resolve (index.js:1956:25)
    at _EditorState.create (index.js:2787:43)
    at sql_editor.tsx:44:31
    at commitHookEffectListMount (react-dom.development.js:23150:26)
    at commitLayoutEffectOnFiber (react-dom.development.js:23268:17)

onewland avatar Feb 04 '25 00:02 onewland

👋🏻 the important parts of the custom extensions are

  1. multiNewLine which allows multiple consecutive newlines. Otherwise CodeMirror collapses them for some reason
  2. sql() for specific syntax highlighting
  3. updateListener to send the text changes upwards in React. This lets the parent React component take the updated text and store it in localStorage with a specific key, so all editors save their state independently from each other

If the basic setup (or minimal) are able to do all that, you're good to go! At the time when I put this together I don't think it did

gggritso avatar Feb 04 '25 16:02 gggritso