Merge branch 'main' into aka/fix-app-embed-lifecycle

.pre-commit-config.yaml

@@ -25,12 +25,12 @@ repos:
         exclude: ^marimo/_tutorials/.*\.md
 
   - repo: https://github.com/crate-ci/typos
-    rev: dictgen-v0.3.1
+    rev: typos-dict-v0.12.4
     hooks:
       - id: typos
 
   - repo: https://github.com/astral-sh/ruff-pre-commit
-    rev: v0.9.3
+    rev: v0.9.4
     hooks:
       # Run the linter
       - id: ruff

docs/guides/working_with_data/sql.md

@@ -153,7 +153,7 @@ duckdb_conn = duckdb.connect("file.db")
 
 <div align="center">
   <figure>
-    <img src="/_static/docs-sql-engine-dropdown.png"/>
+    <img width="750" src="/_static/docs-sql-engine-dropdown.png"/>
     <figcaption>Choose a custom database connection</figcaption>
   </figure>
 </div>

frontend/src/components/app-config/user-config-form.tsx

@@ -1087,27 +1087,6 @@ export const UserConfigForm: React.FC = () => {
                 </div>
               )}
             />
-            <FormField
-              control={form.control}
-              name="experimental.sql_engines"
-              render={({ field }) => (
-                <div className="flex flex-col gap-y-1">
-                  <FormItem className={formItemClasses}>
-                    <FormLabel className="font-normal">SQL Engine</FormLabel>
-                    <FormControl>
-                      <Checkbox
-                        data-testid="sqlengine-sidebar-checkbox"
-                        checked={field.value === true}
-                        onCheckedChange={field.onChange}
-                      />
-                    </FormControl>
-                  </FormItem>
-                  <FormDescription>
-                    Define your own engine and use it in an SQL cell.
-                  </FormDescription>
-                </div>
-              )}
-            />
             {!isWasm() && (
               <FormField
                 control={form.control}

frontend/src/components/databases/display.tsx

@@ -50,3 +50,11 @@ export function dbDisplayName(name: string) {
       return name;
   }
 }
+
+export function transformDisplayName(displayName: string): string {
+  const [dbName, engineName] = displayName.split(" ");
+  if (!engineName) {
+    return dbDisplayName(displayName);
+  }
+  return `${dbDisplayName(dbName)} ${engineName}`;
+}

frontend/src/components/editor/cell/code/cell-editor.tsx

@@ -258,6 +258,7 @@ const CellEditorInternal = ({
     sendToTop,
     sendToBottom,
     splitCell,
+    createNewCell,
     toggleHideCode,
     updateCellCode,
     handleDelete,

frontend/src/components/static-html/share-modal.tsx

@@ -6,7 +6,7 @@ import {
   DialogTitle,
   DialogDescription,
 } from "@/components/ui/dialog";
-import React, { useRef, useState } from "react";
+import React, { useMemo, useState } from "react";
 import { Button } from "@/components/ui/button";
 import { toast } from "@/components/ui/use-toast";
 import { Input } from "../ui/input";
@@ -25,7 +25,7 @@ export const ShareStaticNotebookModal: React.FC<{
 }> = ({ onClose }) => {
   const [slug, setSlug] = useState("");
   // 4 character random string
-  const randomHash = useRef(Math.random().toString(36).slice(2, 6)).current;
+  const randomHash = useMemo(() => Math.random().toString(36).slice(2, 6), []);
 
   // Globally unique path
   const path = `${slug}-${randomHash}`;

frontend/src/core/codemirror/language/__tests__/sql.test.ts

@@ -1,13 +1,13 @@
 /* Copyright 2024 Marimo. All rights reserved. */
 import { expect, describe, it, beforeAll, afterAll, afterEach } from "vitest";
-import {
-  DEFAULT_ENGINE,
-  latestEngineSelected,
-  SQLLanguageAdapter,
-} from "../sql";
+import { SQLLanguageAdapter } from "../sql";
 import { store } from "@/core/state/jotai";
 import { capabilitiesAtom } from "@/core/config/capabilities";
-import type { ConnectionName } from "@/core/cells/data-source-connections";
+import {
+  dataSourceConnectionsAtom,
+  DEFAULT_ENGINE,
+  type ConnectionName,
+} from "@/core/datasets/data-source-connections";
 
 const adapter = new SQLLanguageAdapter();
 
@@ -410,6 +410,25 @@ _df = mo.sql(
   describe("latestEngineSelected", () => {
     afterEach(() => {
       adapter.engine = DEFAULT_ENGINE;
+      const state = store.get(dataSourceConnectionsAtom);
+      const connections = new Map(state.connectionsMap);
+      connections
+        .set("postgres_engine" as ConnectionName, {
+          name: "postgres_engine" as ConnectionName,
+          source: "postgres",
+          display_name: "PostgreSQL",
+          dialect: "postgres",
+        })
+        .set("mysql_engine" as ConnectionName, {
+          name: "mysql_engine" as ConnectionName,
+          source: "mysql",
+          display_name: "MySQL",
+          dialect: "mysql",
+        });
+      store.set(dataSourceConnectionsAtom, {
+        ...state,
+        connectionsMap: connections,
+      });
     });
 
     it("should use default engine initially", () => {
@@ -420,7 +439,9 @@ _df = mo.sql(
       const engine = "postgres_engine" as ConnectionName;
       adapter.selectEngine(engine);
       expect(adapter.engine).toBe(engine);
-      expect(store.get(latestEngineSelected)).toBe(engine);
+      expect(store.get(dataSourceConnectionsAtom).latestEngineSelected).toBe(
+        engine,
+      );
     });
 
     it("should allow switching between engines", () => {
@@ -429,18 +450,40 @@ _df = mo.sql(
 
       adapter.selectEngine(engine1);
       expect(adapter.engine).toBe(engine1);
-      expect(store.get(latestEngineSelected)).toBe(engine1);
+      expect(store.get(dataSourceConnectionsAtom).latestEngineSelected).toBe(
+        engine1,
+      );
 
       adapter.selectEngine(engine2);
       expect(adapter.engine).toBe(engine2);
-      expect(store.get(latestEngineSelected)).toBe(engine2);
+      expect(store.get(dataSourceConnectionsAtom).latestEngineSelected).toBe(
+        engine2,
+      );
     });
 
     it("should update engine in transformIn when specified", () => {
       const pythonCode = '_df = mo.sql("""SELECT 1""", engine=postgres_engine)';
       adapter.transformIn(pythonCode);
       expect(adapter.engine).toBe("postgres_engine");
-      expect(store.get(latestEngineSelected)).toBe("postgres_engine");
+      expect(store.get(dataSourceConnectionsAtom).latestEngineSelected).toBe(
+        "postgres_engine",
+      );
+
+      // Don't update for unspecified engine
+      const pythonCode2 = '_df = mo.sql("""SELECT 1""")';
+      adapter.transformIn(pythonCode2);
+      expect(adapter.engine).toBe(DEFAULT_ENGINE);
+      expect(store.get(dataSourceConnectionsAtom).latestEngineSelected).toBe(
+        "postgres_engine",
+      );
+
+      // Don't update for unknown engine
+      const pythonCode3 = '_df = mo.sql("""SELECT 1""", engine=unknown_engine)';
+      adapter.transformIn(pythonCode3);
+      expect(adapter.engine).toBe("unknown_engine");
+      expect(store.get(dataSourceConnectionsAtom).latestEngineSelected).toBe(
+        "postgres_engine",
+      );
     });
 
     it("should maintain engine selection across transformIn/transformOut", () => {
@@ -473,7 +516,9 @@ _df = mo.sql(
       adapter.selectEngine(DEFAULT_ENGINE);
 
       expect(adapter.engine).toBe(DEFAULT_ENGINE);
-      expect(store.get(latestEngineSelected)).toBe(DEFAULT_ENGINE);
+      expect(store.get(dataSourceConnectionsAtom).latestEngineSelected).toBe(
+        DEFAULT_ENGINE,
+      );
     });
   });
 

frontend/src/core/codemirror/language/panel.tsx

@@ -4,15 +4,25 @@ import { languageAdapterState } from "./extension";
 import { SQLLanguageAdapter } from "./sql";
 import { normalizeName } from "@/core/cells/names";
 import { useAutoGrowInputProps } from "@/hooks/useAutoGrowInputProps";
-import { getFeatureFlag } from "@/core/config/feature-flag";
 import {
   type ConnectionName,
   dataConnectionsMapAtom,
-} from "@/core/cells/data-source-connections";
+} from "@/core/datasets/data-source-connections";
 import { useAtomValue } from "jotai";
-import { CircleHelpIcon } from "lucide-react";
+import { AlertCircle, CircleHelpIcon } from "lucide-react";
 import { Tooltip, TooltipProvider } from "@/components/ui/tooltip";
-import { useState } from "react";
+import {
+  Select,
+  SelectContent,
+  SelectGroup,
+  SelectItem,
+  SelectLabel,
+  SelectTrigger,
+  SelectValue,
+} from "@/components/ui/select";
+import { DatabaseLogo } from "@/components/databases/icon";
+import { transformDisplayName } from "@/components/databases/display";
+import { useNonce } from "@/hooks/useNonce";
 
 export const LanguagePanelComponent: React.FC<{
   view: EditorView;
@@ -59,12 +69,10 @@ export const LanguagePanelComponent: React.FC<{
           />
           <span {...spanProps} />
         </label>
-        {getFeatureFlag("sql_engines") && (
-          <SQLEngineSelect
-            languageAdapter={languageAdapter}
-            onChange={triggerUpdate}
-          />
-        )}
+        <SQLEngineSelect
+          languageAdapter={languageAdapter}
+          onChange={triggerUpdate}
+        />
         <label className="flex items-center gap-2 ml-auto">
           <input
             type="checkbox"
@@ -89,49 +97,75 @@ export const LanguagePanelComponent: React.FC<{
   );
 };
 
-const SQLEngineSelect: React.FC<{
+interface SelectProps {
   languageAdapter: SQLLanguageAdapter;
   onChange: (engine: ConnectionName) => void;
-}> = ({ languageAdapter, onChange }) => {
-  // use local state as languageAdapter may not trigger an update
-  const [selectedEngine, setSelectedEngine] = useState(languageAdapter.engine);
+}
+
+const SQLEngineSelect: React.FC<SelectProps> = ({
+  languageAdapter,
+  onChange,
+}) => {
   const connectionsMap = useAtomValue(dataConnectionsMapAtom);
 
+  // Use nonce to force re-render as languageAdapter.engine may not trigger change
+  // If it's disconnected, we display the engine variable.
+  const selectedEngine = languageAdapter.engine;
+  const rerender = useNonce();
+
+  const engineIsDisconnected =
+    selectedEngine && !connectionsMap.has(selectedEngine);
+
+  const handleSelectEngine = (value: string) => {
+    const nextEngine = connectionsMap.get(value as ConnectionName);
+    if (nextEngine) {
+      languageAdapter.selectEngine(nextEngine.name);
+      rerender();
+      onChange(nextEngine.name);
+    }
+  };
+
   return (
     <div className="flex flex-row gap-1 items-center">
-      <select
-        id="sql-engine"
-        name="sql-engine"
-        className="border border-border rounded px-0.5 focus-visible:outline-none focus-visible:ring-1"
-        value={selectedEngine}
-        onChange={(e) => {
-          const nextEngine = e.target.value as ConnectionName;
-          languageAdapter.selectEngine(nextEngine);
-          setSelectedEngine(nextEngine);
-          onChange(nextEngine);
-        }}
-      >
-        {/* Fallback option if an existing option is deleted, 
-        let's users intentionally switch to default if needed */}
-        <option value="None">Choose an option</option>
-        {[...connectionsMap.entries()].map(([key, value]) => (
-          <option key={key} value={value.name}>
-            {value.display_name}
-          </option>
-        ))}
-      </select>
+      <Select value={selectedEngine} onValueChange={handleSelectEngine}>
+        <SelectTrigger className="text-xs border-border !shadow-none !ring-0 h-4.5 px-1.5">
+          <SelectValue placeholder="Select an engine" />
+        </SelectTrigger>
+        <SelectContent>
+          <SelectGroup>
+            <SelectLabel>Database connections</SelectLabel>
+            {engineIsDisconnected && (
+              <SelectItem key={selectedEngine} value={selectedEngine}>
+                <div className="flex items-center gap-1 opacity-50">
+                  <AlertCircle className="h-3 w-3" />
+                  <span className="truncate">
+                    {transformDisplayName(selectedEngine)}
+                  </span>
+                </div>
+              </SelectItem>
+            )}
+            {[...connectionsMap.entries()].map(([key, value]) => (
+              <SelectItem key={key} value={value.name}>
+                <div className="flex items-center gap-1">
+                  <DatabaseLogo className="h-3 w-3" name={value.source} />
+                  <span className="truncate">
+                    {transformDisplayName(value.display_name)}
+                  </span>
+                </div>
+              </SelectItem>
+            ))}
+          </SelectGroup>
+        </SelectContent>
+      </Select>
       <TooltipProvider>
-        <Tooltip
-          content="Find out how to add an SQL engine"
-          delayDuration={200}
-        >
+        <Tooltip content="How to add a database connection" delayDuration={200}>
           <a
             href="http://docs.marimo.io/guides/working_with_data/sql/#connecting-to-a-custom-database"
             target="_blank"
             rel="noreferrer"
           >
             <CircleHelpIcon
-              size={13}
+              size={12}
               className="text-[var(--sky-11)] opacity-60 hover:opacity-100"
             />
           </a>