Skip to content

fix(desktop): remove duplicate HTML5 backend from v2 Workspace#3174

Merged
Kitenite merged 5 commits into
mainfrom
remove-duplicate-html5-backend
Apr 4, 2026
Merged

fix(desktop): remove duplicate HTML5 backend from v2 Workspace#3174
Kitenite merged 5 commits into
mainfrom
remove-duplicate-html5-backend

Conversation

@Kitenite
Copy link
Copy Markdown
Collaborator

@Kitenite Kitenite commented Apr 4, 2026

Summary

  • Removes the nested DndProvider from packages/panes Workspace component that conflicted with the one in the authenticated layout, causing "Cannot have two HTML5 backends at the same time" when v2 feature flag is disabled
  • Removes unused react-dnd-html5-backend dependency from @superset/panes — child components (TabItem, TabBar, Pane, PaneHeader) use useDrag/useDrop hooks which consume the existing DndProvider context from the layout

Test plan

  • Disable v2 feature flag and verify no "Cannot have two HTML5 backends" error
  • Verify tab drag-and-drop reordering still works in v2 workspace
  • Verify pane drag-and-drop still works in v2 workspace

Summary by cubic

Removed the duplicate HTML5 DnD backend from Workspace to reuse the app-level provider and stop the “Cannot have two HTML5 backends” error when the v2 flag is off; bulk tab close actions now await async closeTab and DnD still works. Updated @superset/panes README to document that Workspace must be wrapped in a single app-level DndProvider.

  • Bug Fixes

    • Removed nested DndProvider/HTML5Backend from packages/panes/src/react/components/Workspace/Workspace.tsx to use the authenticated layout provider.
    • onCloseAllTabs and onCloseOtherTabs now await async closeTab to avoid race conditions and inconsistent state.
  • Dependencies

    • Removed react-dnd-html5-backend from @superset/panes.

Written for commit a8af7cb. Summary will update on new commits.

Summary by CodeRabbit

  • Refactor

    • Workspace no longer provides an internal drag-and-drop context; child components must be wrapped by an external DnD provider.
    • Tab closing now processes each tab sequentially (awaits each close), changing the timing and ordering of multi-tab close actions.
  • Chores

    • Removed the package's declared drag-and-drop backend dependency.
  • Documentation

    • Added usage notes explaining the external DnD provider requirement.

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Apr 4, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a0a55565-46f4-4f58-85bb-5856b2523055

📥 Commits

Reviewing files that changed from the base of the PR and between 256353c and a8af7cb.

📒 Files selected for processing (2)
  • packages/panes/README.md
  • packages/panes/src/react/components/Workspace/Workspace.tsx

📝 Walkthrough

Walkthrough

Removed the bundled react-dnd HTML5 backend and the internal DnDProvider wrapper from Workspace; documentation updated to require consumers to provide a DndProvider when using . (50 words)

Changes

Cohort / File(s) Summary
Dependency Removal
packages/panes/package.json
Removed the react-dnd-html5-backend dependency entry.
Workspace component
packages/panes/src/react/components/Workspace/Workspace.tsx
Removed DndProvider/HTML5Backend wrapper and import; TabBar and tab contents render outside an internal DnDProvider; onCloseOtherTabs/onCloseAllTabs converted to async/await per-tab close.
Documentation
packages/panes/README.md
Added "Drag-and-Drop" section documenting that consumers must wrap <Workspace> with a DndProvider (e.g., HTML5Backend).

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Poem

🐰 I hopped in code to lend a paw,
I nudged the provider from the draw,
Tell the caller, "Bring your net,"
Tabs now dance without a let,
A tiny hop — my whiskers thaw.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly identifies the main change: removing a duplicate HTML5 backend from the v2 Workspace component.
Description check ✅ Passed The PR description is comprehensive and well-structured, covering the problem, solution, dependencies changed, and test plan with clear verification steps.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch remove-duplicate-html5-backend

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Kitenite Kitenite changed the title fix(desktop): remove duplicate HTML5 backend from Workspace fix(desktop): remove duplicate HTML5 backend from v2 Workspace Apr 4, 2026
@greptile-apps
Copy link
Copy Markdown

greptile-apps Bot commented Apr 4, 2026

Greptile Summary

This PR fixes a runtime crash — "Cannot have two HTML5 backends at the same time" — by removing the redundant DndProvider/HTML5Backend wrapper that Workspace was adding on top of the singleton DndProvider already supplied by AuthenticatedLayout.

Key changes:

  • Workspace.tsx: removes the DndProvider + HTML5Backend wrapper; child components (TabItem, TabBar, Pane, PaneHeader) already consume useDrag/useDrop from the existing React DnD context provided by the layout, so drag-and-drop behaviour is unchanged.
  • packages/panes/package.json + bun.lock: removes the now-unused react-dnd-html5-backend dependency; react-dnd itself is correctly retained for the hooks.

Architecture note: @superset/panes is a private package consumed exclusively within the _authenticated route tree, where the singleton dragDropManager (created in renderer/lib/dnd.ts) is always present. The implicit requirement for a parent DndProvider is therefore safe in the current codebase, though worth capturing in package documentation if the package is ever made reusable outside this layout.

Confidence Score: 5/5

  • Safe to merge — the change is a targeted, well-scoped removal of a duplicate DnD provider with no logic side-effects.
  • The fix is minimal and correct: the Workspace component's child hooks (useDrag/useDrop) continue to work because the singleton DndProvider from AuthenticatedLayout is always present in the component tree. The removed react-dnd-html5-backend dependency was the only new import, and it is gone cleanly from both package.json and bun.lock. No test changes are required since the store tests are unaffected. The @superset/panes package is private and its sole consumer lives inside the authenticated layout, so the implicit DnD context dependency is bounded and safe.
  • No files require special attention.

Important Files Changed

Filename Overview
packages/panes/src/react/components/Workspace/Workspace.tsx Removes the duplicate DndProvider/HTML5Backend wrapper; drag-and-drop hooks in child components continue to consume the singleton DndProvider supplied by AuthenticatedLayout.
packages/panes/package.json Removes the now-unused react-dnd-html5-backend dependency; react-dnd is correctly retained for useDrag/useDrop hooks.
bun.lock Lock-file updated to reflect the removed react-dnd-html5-backend entry for @superset/panes.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A["AuthenticatedLayout\n(layout.tsx)"] -->|"DndProvider\nmanager=dragDropManager\n(HTML5Backend singleton)"| B["Outlet / Child Routes"]
    B --> C["V2WorkspacePage\n(page.tsx)"]
    C --> D["Workspace\n(@superset/panes)"]
    D --> E["TabBar"]
    D --> F["Tab"]
    E --> G["TabItem\n(useDrag/useDrop)"]
    F --> H["Pane / PaneHeader\n(useDrag/useDrop)"]

    style A fill:#4ade80,color:#000
    style D fill:#60a5fa,color:#000

    subgraph "BEFORE (conflicting)"
        OLD_D["Workspace\n(DndProvider + HTML5Backend)"]
        OLD_D -->|"❌ Second HTML5Backend\ncauses runtime error"| OLD_PARENT["Parent DndProvider\nalready active"]
    end

    subgraph "AFTER (this PR)"
        NEW_D["Workspace\n(no DndProvider)"]
        NEW_D -->|"✅ Consumes parent\nDndProvider context"| NEW_PARENT["AuthenticatedLayout\nDndProvider"]
    end
Loading

Reviews (1): Last reviewed commit: "Remove provider from v2 workspace" | Re-trigger Greptile

@github-actions
Copy link
Copy Markdown
Contributor

github-actions Bot commented Apr 4, 2026

🚀 Preview Deployment

🔗 Preview Links

Service Status Link
Neon Database (Neon) View Branch
Fly.io Electric (Fly.io) View App
Vercel API (Vercel) Open Preview
Vercel Web (Vercel) Open Preview
Vercel Marketing (Vercel) Open Preview
Vercel Admin (Vercel) Open Preview
Vercel Docs (Vercel) Open Preview

Preview updates automatically with new commits

Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai Bot left a comment

Choose a reason for hiding this comment

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

No issues found across 3 files

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@packages/panes/src/react/components/Workspace/Workspace.tsx`:
- Around line 43-51: The bulk tab-close handlers (onCloseOtherTabs and
onCloseAllTabs) call the async closeTab for each tab without awaiting, causing
concurrent onBeforeCloseTab flows; change these callbacks to async and iterate
with a for...of loop awaiting closeTab(tab.id) for each tab (use the existing
tabs array and closeTab function), ensuring each close completes before
proceeding to the next to avoid race conditions in validation/store updates.
- Around line 38-77: The bulk-close callbacks currently call the async function
closeTab without awaiting it in onCloseOtherTabs and onCloseAllTabs, which can
produce overlapping operations and unhandled rejections from onBeforeCloseTab;
make those callbacks async and either await each closeTab sequentially (for
predictable ordering) or collect promises and await Promise.allSettled to handle
errors; update the onCloseOtherTabs and onCloseAllTabs handlers to await
closeTab for each tab and add basic error handling (try/catch or allSettled) so
rejections from onBeforeCloseTab are handled gracefully.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 839bf85f-67d8-4985-b8a0-e30281f8b8a7

📥 Commits

Reviewing files that changed from the base of the PR and between 7599ace and d7d6ad7.

⛔ Files ignored due to path filters (1)
  • bun.lock is excluded by !**/*.lock
📒 Files selected for processing (2)
  • packages/panes/package.json
  • packages/panes/src/react/components/Workspace/Workspace.tsx
💤 Files with no reviewable changes (1)
  • packages/panes/package.json

Comment thread packages/panes/src/react/components/Workspace/Workspace.tsx
Comment thread packages/panes/src/react/components/Workspace/Workspace.tsx Outdated
@Kitenite Kitenite merged commit 4d7c612 into main Apr 4, 2026
14 checks passed
MocA-Love pushed a commit to MocA-Love/superset that referenced this pull request Apr 5, 2026
…set-sh#3174)

* Reuse dnd backend

* Remove provider from v2 workspace

* Lint

* fix: await async closeTab in bulk close callbacks

* docs(panes): document DndProvider requirement for consumers
MocA-Love added a commit to MocA-Love/superset that referenced this pull request Apr 5, 2026
cherry-pick方式で内容を取り込み済みの14コミットをgit履歴上もマージ済みにする。

取り込み済み (cherry-pick / 手動移植):
- be22b46 superset-sh#3125 — スキップ (下記参照)
- 88bc7fb superset-sh#3127 — Revert DA1 ✓
- 92d0ff9 superset-sh#3054 — DA1 fix ✓
- c48450e superset-sh#3093 — file viewer pane fix ✓
- fffa8db superset-sh#3128 — version 1.4.7 ✓
- 589a7c7 superset-sh#3136 — fuzzy scorer (ハイブリッド方式) ✓
- ceb8c81 superset-sh#3150 — Electron 40.8.5 ✓
- 8922b94 superset-sh#3137 — terminalId分離 ✓
- c7508e5 superset-sh#3152 — GitHub無料化 ✓
- 2b91f11 superset-sh#3155 — v2 terminal theme ✓
- b8b11af superset-sh#3154 — TUI dimension fix ✓
- 7599ace superset-sh#3149 — v2 sidebar file tree (手動統合) ✓
- 4d7c612 superset-sh#3174 — DnD重複削除 ✓
- 864977d superset-sh#3157 — Host Service分離 ✓

意図的にスキップ:
- be22b46 superset-sh#3125 (GitHub polling簡素化)
  フォーク独自のGitHubSyncService (バックエンド集中ポーリング) と
  設計が異なるため不採用。upstreamはフロントエンドhover駆動、フォークは
  バックエンドキャッシュウォーマー方式。詳細は githubQueryPolicy.ts と
  github-sync-service.ts のFORK NOTEを参照。

ゴースト・マージ復元 (revert 134cfd5 で消失した内容):
- 538f306 superset-sh#3120 — Patch vuln ✓
- 1588d20 superset-sh#3108 — terminal lifecycle分離 ✓
- 59426f6 superset-sh#3122 — file tree + FilePane + Alert refactor (手動統合) ✓
- 10d9a5d superset-sh#3097 — tiptap line-height ✓
- 337a9ae superset-sh#3121 — Codex hooks削除 ✓
@Kitenite Kitenite deleted the remove-duplicate-html5-backend branch April 13, 2026 16:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant