fix: monaco flicker pr test - #344

packages/sql-editor/src/SqlMonacoEditor.tsx (1)

316-336: 🛠️ Refactor suggestion | 🟠 Major

Stale closure risk when model changes after prop updates.

The setContextForModel function captures prop values (connector, tableSchemas, etc.) at mount time. If props change after mount and then the path changes (triggering onDidChangeModel), the new model receives stale context from the initial closure rather than the current prop values.

Sequence demonstrating the issue:

1. Mount captures connector = C1
2. connector prop updates to C2 → useEffect updates current model
3. path changes → onDidChangeModel fires → new model gets C1 (stale)

Consider using a ref to track the latest prop values so onDidChangeModel always sets current context:

Proposed fix using a ref for latest props

 export const SqlMonacoEditor: React.FC<SqlMonacoEditorProps> = ({
   connector,
   customKeywords = [],
   customFunctions = [],
   tableSchemas = [],
   getLatestSchemas,
   onMount,
   className,
   options,
   ...restProps
 }) => {
   const modelRef = useRef<any>(null);
+  const latestPropsRef = useRef({
+    connector,
+    tableSchemas,
+    getLatestSchemas,
+    customKeywords,
+    customFunctions,
+  });
+
+  // Keep ref in sync with latest props
+  useEffect(() => {
+    latestPropsRef.current = {
+      connector,
+      tableSchemas,
+      getLatestSchemas,
+      customKeywords,
+      customFunctions,
+    };
+  }, [connector, tableSchemas, getLatestSchemas, customKeywords, customFunctions]);

Then inside handleEditorDidMount:

-      const setContextForModel = (model: any) => {
-        if (!model) return;
-        modelRef.current = model;
-        sqlCompletionContextByModel.set(model, {
-          connector,
-          tableSchemas,
-          getLatestSchemas,
-          customKeywords,
-          customFunctions,
-        });
-      };
+      const setContextForModel = (model: any) => {
+        if (!model) return;
+        modelRef.current = model;
+        sqlCompletionContextByModel.set(model, latestPropsRef.current);
+      };