Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions apps/desktop/src/renderer/hooks/useFileDragBehavior/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
export {
getFileDragBehavior,
useFileDragBehavior,
} from "./useFileDragBehavior";
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
import type { FileDragBehavior } from "@superset/local-db";
import { electronTrpc } from "renderer/lib/electron-trpc";
import { DEFAULT_FILE_DRAG_BEHAVIOR } from "shared/constants";

let cachedFileDragBehavior: FileDragBehavior = DEFAULT_FILE_DRAG_BEHAVIOR;

/** Non-React getter, kept in sync by useFileDragBehavior(). */
export function getFileDragBehavior(): FileDragBehavior {
return cachedFileDragBehavior;
}

export function useFileDragBehavior(): FileDragBehavior {
const { data } = electronTrpc.settings.getFileDragBehavior.useQuery();
const behavior = data ?? DEFAULT_FILE_DRAG_BEHAVIOR;
cachedFileDragBehavior = behavior;
Comment on lines +12 to +15
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Apr 19, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

🧩 Analysis chain

🏁 Script executed:

# First, let's locate and examine the file
fd -t f "useFileDragBehavior.ts" apps/desktop

Repository: MocA-Love/superset

Length of output: 136


🏁 Script executed:

# Read the complete file to see the context
cat -n apps/desktop/src/renderer/hooks/useFileDragBehavior/useFileDragBehavior.ts

Repository: MocA-Love/superset

Length of output: 836


🏁 Script executed:

# Search for where cachedFileDragBehavior is defined and used
rg "cachedFileDragBehavior" apps/desktop/src/renderer/hooks/useFileDragBehavior/

Repository: MocA-Love/superset

Length of output: 429


🏁 Script executed:

# Check the referenced useFileOpenMode pattern
fd -t f "useFileOpenMode*" apps/desktop/src/renderer/hooks/

Repository: MocA-Love/superset

Length of output: 128


🏁 Script executed:

# Look for how getFileDragBehavior is exported and used outside React
rg "getFileDragBehavior" apps/desktop/src/renderer/hooks/useFileDragBehavior/

Repository: MocA-Love/superset

Length of output: 424


🏁 Script executed:

# Check the useFileOpenMode pattern to see if it has a similar pattern
cat -n apps/desktop/src/renderer/hooks/useFileOpenMode/useFileOpenMode.ts

Repository: MocA-Love/superset

Length of output: 768


🏁 Script executed:

# Look for where getFileDragBehavior is called outside of React components
rg "getFileDragBehavior\(\)" apps/desktop/src --type ts --type tsx

Repository: MocA-Love/superset

Length of output: 89


🏁 Script executed:

# Check if there are any side effects or IPC calls that use getFileDragBehavior
rg "getFileDragBehavior" apps/desktop/src -A 2 -B 2

Repository: MocA-Love/superset

Length of output: 6093


レンダー中のモジュールキャッシュ更新を避けてください。

Line 15 でレンダー実行中に外部可変状態 cachedFileDragBehavior を書き換えると、React の未コミットレンダー値が getFileDragBehavior() に漏れる可能性があります。useEffect で同期することで、コミット後の状態のみを反映できます。

修正案
+import { useEffect } from "react";
 import type { FileDragBehavior } from "@superset/local-db";
 import { electronTrpc } from "renderer/lib/electron-trpc";
 import { DEFAULT_FILE_DRAG_BEHAVIOR } from "shared/constants";
@@
 export function useFileDragBehavior(): FileDragBehavior {
 	const { data } = electronTrpc.settings.getFileDragBehavior.useQuery();
 	const behavior = data ?? DEFAULT_FILE_DRAG_BEHAVIOR;
-	cachedFileDragBehavior = behavior;
+	useEffect(() => {
+		cachedFileDragBehavior = behavior;
+	}, [behavior]);
 	return behavior;
 }
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@apps/desktop/src/renderer/hooks/useFileDragBehavior/useFileDragBehavior.ts`
around lines 12 - 15, The hook useFileDragBehavior currently mutates the
external mutable cachedFileDragBehavior during render; change this to update the
cache inside a useEffect so the write occurs after commit. Keep using
electronTrpc.settings.getFileDragBehavior.useQuery() and the local behavior =
data ?? DEFAULT_FILE_DRAG_BEHAVIOR, then add a useEffect that runs when behavior
changes and assigns cachedFileDragBehavior = behavior inside the effect to avoid
writing during render and leaking uncommitted values to getFileDragBehavior().

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

この件は今回は見送ります。理由は 2 点で、まず getFileDragBehavior() は現状どこからも使われておらず、未コミット値が外へ漏れる実害がありません。加えて useFileOpenMode()useRightSidebarOpenViewWidth() も同じモジュールキャッシュ更新パターンを使っているため、この hook だけ useEffect 化すると設定 getter 群の振る舞いが不揃いになります。もし直すなら 3 つまとめて揃える follow-up の方が筋だと判断しました。

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@MocA-Love、ご説明ありがとうございます。理解しました。getFileDragBehavior() が未使用である点と、useFileOpenMode() / useRightSidebarOpenViewWidth() と設計を揃えるという判断は筋が通っています。3 つまとめて修正する follow-up の方が一貫性の観点からも適切ですね。

今後の follow-up 用に学習として記録しておきます。follow-up issue を作成しましょうか?


✏️ Learnings added
Learnt from: MocA-Love
Repo: MocA-Love/superset PR: 336
File: apps/desktop/src/renderer/hooks/useFileDragBehavior/useFileDragBehavior.ts:12-15
Timestamp: 2026-04-19T01:06:02.134Z
Learning: In `apps/desktop/src/renderer/hooks/`, the hooks `useFileDragBehavior`, `useFileOpenMode`, and `useRightSidebarOpenViewWidth` all intentionally use the same module-level cache update pattern (assigning to a module-level variable directly during render, not inside a `useEffect`). This is a deliberate design choice for consistency across settings-getter hooks. If this pattern is changed (e.g., to use `useEffect`), all three hooks must be updated together in the same PR.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


🧠 Learnings used
Learnt from: CR
Repo: MocA-Love/superset PR: 0
File: apps/desktop/AGENTS.md:0-0
Timestamp: 2026-03-27T17:12:04.711Z
Learning: Applies to apps/desktop/**/*.{ts,tsx} : For Electron interprocess communication, ALWAYS use tRPC as defined in `src/lib/trpc`

return behavior;
}
8 changes: 7 additions & 1 deletion apps/desktop/src/renderer/lib/file-drag.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
export const INTERNAL_FILE_PATH_MIME = "application/x-superset-file-path";

export function hasInternalDraggedFilePath(
dataTransfer: DataTransfer,
): boolean {
return Array.from(dataTransfer.types).includes(INTERNAL_FILE_PATH_MIME);
}

export function getInternalDraggedFilePath(
dataTransfer: DataTransfer,
): string | null {
if (!Array.from(dataTransfer.types).includes(INTERNAL_FILE_PATH_MIME)) {
if (!hasInternalDraggedFilePath(dataTransfer)) {
return null;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
import { cn } from "@superset/ui/utils";
import { useContext, useRef } from "react";
import { useCallback, useContext, useEffect, useRef, useState } from "react";
import type { MosaicBranch } from "react-mosaic-component";
import { MosaicWindow, MosaicWindowContext } from "react-mosaic-component";
import { useFileDragBehavior } from "renderer/hooks/useFileDragBehavior";
import {
getInternalDraggedFilePath,
hasInternalDraggedFilePath,
} from "renderer/lib/file-drag";
import { useDragPaneStore } from "renderer/stores/drag-pane-store";
import { useTabsStore } from "renderer/stores/tabs/store";
import type { MosaicDropPosition } from "renderer/stores/tabs/types";
import type { SplitOrientation } from "../../hooks";
import { useSplitOrientation } from "../../hooks";
import { InternalFileDropOverlay } from "./components/InternalFileDropOverlay";

export interface PaneHandlers {
onFocus: () => void;
Expand All @@ -16,6 +23,23 @@ export interface PaneHandlers {
splitOrientation: SplitOrientation;
}

function getInternalFileDropPosition(
clientX: number,
clientY: number,
rect: DOMRect,
): MosaicDropPosition {
const cx = rect.left + rect.width / 2;
const cy = rect.top + rect.height / 2;
const dx = clientX - cx;
const dy = clientY - cy;

if (Math.abs(dx) > Math.abs(dy)) {
return dx > 0 ? "right" : "left";
}

return dy > 0 ? "bottom" : "top";
}

/**
* Connects drag source for root panes (single pane in a tab).
* react-mosaic-component skips drag connection for root panes (path=[]),
Expand Down Expand Up @@ -76,16 +100,25 @@ export function BasePaneWindow({
hideToolbar = false,
}: BasePaneWindowProps) {
const isActive = useTabsStore((s) => s.focusedPaneIds[tabId] === paneId);
const workspaceId = useTabsStore(
(s) => s.tabs.find((tab) => tab.id === tabId)?.workspaceId ?? null,
);
const workspaceRunState = useTabsStore(
(s) => s.panes[paneId]?.workspaceRun?.state,
);
const addFileViewerPane = useTabsStore((s) => s.addFileViewerPane);
const containerRef = useRef<HTMLDivElement>(null);
const splitOrientation = useSplitOrientation(containerRef);
const isDragging = useDragPaneStore((s) => s.draggingPaneId !== null);
const isTabDragging = useDragPaneStore((s) => s.isTabDragging);
const isResizing = useDragPaneStore((s) => s.isResizing);
const setDragging = useDragPaneStore((s) => s.setDragging);
const clearDragging = useDragPaneStore((s) => s.clearDragging);
const fileDragBehavior = useFileDragBehavior();
const internalFileDragDepthRef = useRef(0);
const internalFileDropPositionRef = useRef<MosaicDropPosition | null>(null);
const [internalFileDropPosition, setInternalFileDropPosition] =
useState<MosaicDropPosition | null>(null);

const handleFocus = () => {
setFocusedPane(tabId, paneId);
Expand Down Expand Up @@ -135,6 +168,140 @@ export function BasePaneWindow({

const isRoot = path.length === 0;

const resetInternalFileDragState = useCallback(() => {
internalFileDragDepthRef.current = 0;
internalFileDropPositionRef.current = null;
setInternalFileDropPosition(null);
}, []);

const isInternalFileViewerDropEnabled =
fileDragBehavior === "open-file-viewer" && workspaceId !== null;

const updateInternalFileDropPosition = useCallback(
(event: React.DragEvent<HTMLDivElement>): MosaicDropPosition => {
const nextPosition = getInternalFileDropPosition(
event.clientX,
event.clientY,
event.currentTarget.getBoundingClientRect(),
);

if (nextPosition !== internalFileDropPositionRef.current) {
internalFileDropPositionRef.current = nextPosition;
setInternalFileDropPosition(nextPosition);
}

return nextPosition;
},
[],
);

useEffect(() => {
if (!isInternalFileViewerDropEnabled) {
resetInternalFileDragState();
}
}, [isInternalFileViewerDropEnabled, resetInternalFileDragState]);

const handleInternalFileDragEnterCapture = useCallback(
(event: React.DragEvent<HTMLDivElement>) => {
if (
!isInternalFileViewerDropEnabled ||
!hasInternalDraggedFilePath(event.dataTransfer)
) {
return;
}

internalFileDragDepthRef.current += 1;
event.preventDefault();
event.stopPropagation();
event.dataTransfer.dropEffect = "copy";
updateInternalFileDropPosition(event);
},
[isInternalFileViewerDropEnabled, updateInternalFileDropPosition],
);

const handleInternalFileDragOverCapture = useCallback(
(event: React.DragEvent<HTMLDivElement>) => {
if (
!isInternalFileViewerDropEnabled ||
!hasInternalDraggedFilePath(event.dataTransfer)
) {
return;
}

event.preventDefault();
event.stopPropagation();
event.dataTransfer.dropEffect = "copy";
updateInternalFileDropPosition(event);
},
[isInternalFileViewerDropEnabled, updateInternalFileDropPosition],
);

const handleInternalFileDragLeaveCapture = useCallback(
(event: React.DragEvent<HTMLDivElement>) => {
if (
!isInternalFileViewerDropEnabled ||
!hasInternalDraggedFilePath(event.dataTransfer)
) {
return;
}

event.stopPropagation();
internalFileDragDepthRef.current = Math.max(
0,
internalFileDragDepthRef.current - 1,
);

if (internalFileDragDepthRef.current === 0) {
resetInternalFileDragState();
}
},
[isInternalFileViewerDropEnabled, resetInternalFileDragState],
);

const handleInternalFileDropCapture = useCallback(
(event: React.DragEvent<HTMLDivElement>) => {
if (
!isInternalFileViewerDropEnabled ||
!workspaceId ||
!hasInternalDraggedFilePath(event.dataTransfer)
) {
return;
}

event.preventDefault();
event.stopPropagation();

const filePath = getInternalDraggedFilePath(event.dataTransfer);
if (!filePath) {
resetInternalFileDragState();
return;
}

const dropPosition = updateInternalFileDropPosition(event);

addFileViewerPane(workspaceId, {
filePath,
isPinned: true,
openInNewTab: false,
reuseExisting: "none",
relativeToPaneId: paneId,
relativeToTabId: tabId,
relativeSplitPosition: dropPosition,
useRightSidebarOpenViewWidth: true,
});
resetInternalFileDragState();
Comment thread
MocA-Love marked this conversation as resolved.
},
[
addFileViewerPane,
isInternalFileViewerDropEnabled,
paneId,
resetInternalFileDragState,
tabId,
updateInternalFileDropPosition,
workspaceId,
],
);

return (
<MosaicWindow<string>
path={path}
Expand All @@ -157,14 +324,19 @@ export function BasePaneWindow({
{/* biome-ignore lint/a11y/useKeyWithClickEvents lint/a11y/noStaticElementInteractions: Focus handler for pane */}
<div
ref={containerRef}
className={contentClassName}
className={cn("relative", contentClassName)}
style={
isDragging || isTabDragging || isResizing
? { pointerEvents: "none" }
: undefined
}
onDragEnterCapture={handleInternalFileDragEnterCapture}
onDragOverCapture={handleInternalFileDragOverCapture}
onDragLeaveCapture={handleInternalFileDragLeaveCapture}
onDropCapture={handleInternalFileDropCapture}
onClick={handleFocus}
>
<InternalFileDropOverlay position={internalFileDropPosition} />
{children}
</div>
</MosaicWindow>
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
import type { CSSProperties } from "react";
import type { MosaicDropPosition } from "renderer/stores/tabs/types";

interface InternalFileDropOverlayProps {
position: MosaicDropPosition | null;
}

const ZONE_STYLES: Record<MosaicDropPosition, CSSProperties> = {
top: { top: 0, left: 0, width: "100%", height: "50%" },
bottom: { top: "50%", left: 0, width: "100%", height: "50%" },
left: { top: 0, left: 0, width: "50%", height: "100%" },
right: { top: 0, left: "50%", width: "50%", height: "100%" },
};

export function InternalFileDropOverlay({
position,
}: InternalFileDropOverlayProps) {
if (!position) return null;

return (
<div className="pointer-events-none absolute inset-0 z-10">
<div
className="absolute rounded-sm border-2 border-primary/70 bg-primary/10"
style={{
...ZONE_STYLES[position],
transition: "all 150ms ease",
}}
/>
</div>
);
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export { InternalFileDropOverlay } from "./InternalFileDropOverlay";
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,10 @@ import type { Terminal as XTerm } from "@xterm/xterm";
import "@xterm/xterm/css/xterm.css";
import { memo, useCallback, useEffect, useRef, useState } from "react";
import { electronTrpc } from "renderer/lib/electron-trpc";
import { getInternalDraggedFilePath } from "renderer/lib/file-drag";
import { buildTerminalCommand } from "renderer/lib/terminal/launch-command";
import { useTabsStore } from "renderer/stores/tabs/store";
import { useTerminalSuggestionsStore } from "renderer/stores/terminal-suggestions";
import { useEffectiveTerminalTheme } from "renderer/stores/vibrancy";
import { DEFAULT_FILE_DRAG_BEHAVIOR } from "shared/constants";
import { sanitizeForTitle } from "./commandBuffer";
import { SessionKilledOverlay } from "./components";
import {
Expand Down Expand Up @@ -56,9 +54,6 @@ export const Terminal = memo(function Terminal({
const isWorkspaceRunPane = Boolean(pane?.workspaceRun?.workspaceId);
const paneInitialCwd = pane?.initialCwd;
const clearPaneInitialData = useTabsStore((s) => s.clearPaneInitialData);
const addFileViewerPane = useTabsStore((s) => s.addFileViewerPane);
const { data: fileDragBehavior } =
electronTrpc.settings.getFileDragBehavior.useQuery();

const { data: workspaceData } = electronTrpc.workspaces.get.useQuery(
{ id: workspaceId },
Expand Down Expand Up @@ -567,19 +562,6 @@ export const Terminal = memo(function Terminal({
const handleDrop = (event: React.DragEvent) => {
event.preventDefault();
const files = Array.from(event.dataTransfer.files);
const internalFilePath = getInternalDraggedFilePath(event.dataTransfer);
if (
files.length === 0 &&
internalFilePath &&
(fileDragBehavior ?? DEFAULT_FILE_DRAG_BEHAVIOR) === "open-file-viewer"
) {
addFileViewerPane(workspaceId, {
filePath: internalFilePath,
useRightSidebarOpenViewWidth: true,
});
return;
}

let text: string;
if (files.length > 0) {
// Native file drop (from Finder, etc.)
Expand Down
Loading
Loading