fix(sql-lab): prevent crash when host shell lacks useAppDispatch export

[yousoph]

Summary

Fixes TypeError: o.useAppDispatch is not a function when clicking SQL Lab.

• Adds a shim at src/SqlLab/hooks/useAppDispatch.ts that re-exports useAppDispatch from src/views/store with a nullish-coalesce fallback to useDispatch
• Updates all 19 SQL Lab components to import from the shim

Root cause

In Module Federation deployments where the host shell shares src/views/store as a singleton, a version skew between the shell bundle and the SQL Lab remote chunk can leave useAppDispatch undefined at runtime. The shell was built before useAppDispatch was exported (commit 785a08c), but SQL Lab was built after. SQL Lab gets the older shared module and crashes.

Fix

The shim is transparent in up-to-date deployments. In version-skewed deployments it falls back silently to useDispatch rather than crashing.

Test plan

• Click SQL Lab — loads without TypeError: useAppDispatch is not a function
• cd superset-frontend && npm run type-check passes

🤖 Generated with Claude Code

[bito-code-review]

Code Review Agent Run #cd61de

Actionable Suggestions - 0

Additional Suggestions - 2

• superset-frontend/src/SqlLab/hooks/useAppDispatch.ts - 2

  • Silent dispatch type degradation · Line 34-35
    The fallback to `useDispatch` returns `ReactDispatch` without thunk support. All 17 SqlLab components using this hook expect `AppDispatch` with thunk middleware (e.g., async thunk actions). If the fallback fires silently, dispatched thunks will be lost with no runtime indication. Add a runtime warning or error when falling back so this silent type degradation is discoverable.
    Code suggestion

    --- superset-frontend/src/SqlLab/hooks/useAppDispatch.ts (lines 30-35) ---
     30: const maybeUseAppDispatch = _storeUseAppDispatch as
     31:   | (() => AppDispatch)
     32:   | undefined;
     33: 
     34: export const useAppDispatch: () => AppDispatch =
     35:-	maybeUseAppDispatch ?? useDispatch;
    +	maybeUseAppDispatch ?? (() => {
    +	  // eslint-disable-next-line no-console
    +	  console.warn('[SqlLab] useAppDispatch unavailable — falling back to useDispatch without thunk support');
    +	  return useDispatch();
    +	});

  • Missing test coverage for hook · Line 1-35
    The new hook lacks dedicated unit test coverage. With 17 SqlLab components depending on this hook, a regression in the fallback logic could silently break SQL Lab dispatch in Module Federation deployments. Add tests exercising both the primary and fallback paths.

Review Details

• Files reviewed - 20 · Commit Range: 69cc9e2..69cc9e2

  • superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx
  • superset-frontend/src/SqlLab/components/EditorWrapper/index.tsx
  • superset-frontend/src/SqlLab/components/EditorWrapper/useKeywords.ts
  • superset-frontend/src/SqlLab/components/ExploreCtasResultsButton/index.tsx
  • superset-frontend/src/SqlLab/components/PopEditorTab/index.tsx
  • superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.tsx
  • superset-frontend/src/SqlLab/components/QueryLimitSelect/index.tsx
  • superset-frontend/src/SqlLab/components/QueryTable/index.tsx
  • superset-frontend/src/SqlLab/components/ResultSet/index.tsx
  • superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx
  • superset-frontend/src/SqlLab/components/SouthPane/index.tsx
  • superset-frontend/src/SqlLab/components/SqlEditor/index.tsx
  • superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx
  • superset-frontend/src/SqlLab/components/SqlEditorTabHeader/index.tsx
  • superset-frontend/src/SqlLab/components/SqlEditorTopBar/useDatabaseSelector.ts
  • superset-frontend/src/SqlLab/components/TableElement/index.tsx
  • superset-frontend/src/SqlLab/components/TableExploreTree/index.tsx
  • superset-frontend/src/SqlLab/components/TableExploreTree/useTreeData.ts
  • superset-frontend/src/SqlLab/components/TablePreview/index.tsx
  • superset-frontend/src/SqlLab/hooks/useAppDispatch.ts
• Files skipped - 0
• Tools

  • Whispers (Secret Scanner) - ✔︎ Successful
  • Detect-secrets (Secret Scanner) - ✔︎ Successful
  • Eslint (Linter) - ✔︎ Successful

---

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

• /review - Manually triggers a full AI review.

• /pause - Pauses automatic reviews on this pull request.

• /resume - Resumes automatic reviews.

• /resolve - Marks all Bito-posted review comments as resolved.

• /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

• Customize agent settings
• Review rules
• General documentation
• FAQ

AI Code Review powered by

[Copilot]

Pull request overview

This PR prevents SQL Lab from crashing in Module Federation deployments when the host shell provides an older shared src/views/store module that does not export useAppDispatch, by routing SQL Lab’s dispatch hook through a local shim that can fall back to react-redux’s useDispatch.

Changes:

• Added src/SqlLab/hooks/useAppDispatch.ts shim that re-exports useAppDispatch from src/views/store with a runtime fallback to useDispatch.
• Updated SQL Lab components/hooks to import useAppDispatch from the shim instead of src/views/store.
• Kept typed dispatch (AppDispatch) as the public hook return type for SQL Lab call sites.

Reviewed changes

Copilot reviewed 20 out of 20 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
superset-frontend/src/SqlLab/hooks/useAppDispatch.ts	Introduces a guarded useAppDispatch export with a useDispatch fallback for version-skewed shared store modules.
superset-frontend/src/SqlLab/components/TablePreview/index.tsx	Switches useAppDispatch import to the SQL Lab shim.
superset-frontend/src/SqlLab/components/TableExploreTree/useTreeData.ts	Switches useAppDispatch import to the SQL Lab shim.
superset-frontend/src/SqlLab/components/TableExploreTree/index.tsx	Switches useAppDispatch import to the SQL Lab shim.
superset-frontend/src/SqlLab/components/TableElement/index.tsx	Switches useAppDispatch import to the SQL Lab shim.
superset-frontend/src/SqlLab/components/SqlEditorTopBar/useDatabaseSelector.ts	Switches useAppDispatch import to the SQL Lab shim.
superset-frontend/src/SqlLab/components/SqlEditorTabHeader/index.tsx	Switches useAppDispatch import to the SQL Lab shim.
superset-frontend/src/SqlLab/components/SqlEditorLeftBar/index.tsx	Switches useAppDispatch import to the SQL Lab shim.
superset-frontend/src/SqlLab/components/SqlEditor/index.tsx	Switches useAppDispatch import to the SQL Lab shim.
superset-frontend/src/SqlLab/components/SouthPane/index.tsx	Switches useAppDispatch import to the SQL Lab shim.
superset-frontend/src/SqlLab/components/SaveDatasetModal/index.tsx	Splits selector/dispatch imports so useAppDispatch comes from the shim.
superset-frontend/src/SqlLab/components/ResultSet/index.tsx	Switches useAppDispatch import to the SQL Lab shim.
superset-frontend/src/SqlLab/components/QueryTable/index.tsx	Switches useAppDispatch import to the SQL Lab shim.
superset-frontend/src/SqlLab/components/QueryLimitSelect/index.tsx	Switches useAppDispatch import to the SQL Lab shim.
superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.tsx	Switches useAppDispatch import to the SQL Lab shim.
superset-frontend/src/SqlLab/components/PopEditorTab/index.tsx	Switches useAppDispatch import to the SQL Lab shim.
superset-frontend/src/SqlLab/components/ExploreCtasResultsButton/index.tsx	Switches useAppDispatch import to the SQL Lab shim.
superset-frontend/src/SqlLab/components/EditorWrapper/useKeywords.ts	Switches useAppDispatch import to the SQL Lab shim.
superset-frontend/src/SqlLab/components/EditorWrapper/index.tsx	Switches useAppDispatch import to the SQL Lab shim.
superset-frontend/src/SqlLab/components/EditorAutoSync/index.tsx	Switches useAppDispatch import to the SQL Lab shim.

[aminghadersohi]

The shim approach is correct and well-scoped. Root cause confirmed: commit 785a08c7d5 ("chore(frontend): export typed useAppDispatch / useAppSelector hooks") added useAppDispatch as a named constant to store.ts; any Module Federation host shell built before that commit will serve a store module object without that key, causing the TypeError on every SQL Lab load.

What's right:

• ?? evaluated at module-init time — correct for a static version-skew condition.
• Fallback to raw useDispatch is safe at runtime: thunk middleware is still active in the store, so thunk dispatch works even without the typed wrapper.
• Shim is in src/SqlLab/ rather than modifying src/views/store — correct layer. Keeps the fallback isolated to the remote chunk and doesn't affect the rest of the codebase.
• All 19 import sites updated consistently.

Two non-blocking suggestions:

1. Consider adding a unit test for the fallback path. The ?? logic is the critical part of this fix, but it's untested. It's possible to exercise it via jest.mock('src/views/store', () => ({})) and asserting the exported useAppDispatch equals useDispatch. Without it, a future refactor that accidentally removes the as | undefined cast could silently break the fallback.

2. Minor type honesty nit: In the fallback path, useDispatch returns Dispatch<AnyAction> but the export types it as () => AppDispatch. Works at runtime (thunk middleware handles it), but explicitly casting useDispatch as () => AppDispatch in the ?? branch would make the intentional narrowing visible rather than implicit.

Both are optional. The fix is correct as-is.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.05%. Comparing base (53e2793) to head (fa49f1c).

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #40506      +/-   ##
==========================================
+ Coverage   64.01%   64.05%   +0.03%     
==========================================
  Files        2594     2594              
  Lines      139898   139681     -217     
  Branches    32470    32445      -25     
==========================================
- Hits        89558    89470      -88     
+ Misses      48795    48673     -122     
+ Partials     1545     1538       -7     

Flag	Coverage Δ
javascript	67.34% <100.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[aminghadersohi]

Reviewed post-approval commit fa49f1c. It is a merge-from-master bringing in unrelated Python soft-delete features and routine dependency bumps — none of the PR's 20 TypeScript files were touched. Prior approval stands. No new concerns.