apache / apache/superset

Migrate to react-redux v8 with a typed AppDispatch / useAppDispatch

Open
#39,927 1 comment 0 reactions 0 assignees View on GitHub
dependencies:npm frontend:refactor
Dominant language
Python
Stars
74.8k
Forks
18.3k
Avg merge
2d 5h
Merged PRs (30d)
685

Description

Sub-issue of #39890.

The `react-redux` v7 → v8+ bump turned out to be larger than the parent issue anticipated. Tracking it separately so it doesn't block the rest of the React 18 concurrent-feature adoption work.

## Why it's not a one-line bump

v8 tightened `useDispatch()` and `connect`'s `Dispatch` typing so it no longer auto-accepts thunks. A representative sample (just `SqlLab/components/EditorAutoSync/index.tsx`, `PopEditorTab/index.tsx`, `QueryTable/index.tsx`) surfaced ~50 new errors of these shapes:

```
error TS2345: Argument of type 'SqlLabThunkAction' is not assignable to parameter of type 'AnyAction'
error TS2344: Type '(dispatch: any) => Promise<…>' does not satisfy the constraint 'Dispatch'
```

Across the full codebase the affected call sites likely number in the hundreds. The proper fix is the redux-toolkit-recommended pattern:

```ts
// in src/views/store/index.ts (or wherever the store is exported)
import { useDispatch, useSelector, type TypedUseSelectorHook } from 'react-redux';
import type { ThunkDispatch } from 'redux-thunk';
import type { AnyAction } from 'redux';

export type AppDispatch = ThunkDispatch;
export const useAppDispatch: () => AppDispatch = useDispatch;
export const useAppSelector: TypedUseSelectorHook = useSelector;
```

…then migrate `useDispatch()` → `useAppDispatch()` and `useSelector(...)` → `useAppSelector(...)` across all consumers.

## Suggested approach

1. **PR 1**: Define the typed hooks alongside the existing store, *don't* migrate any call sites. Verify it compiles and exports cleanly.
2. **PR 2…N**: Migrate one feature area per PR (SqlLab, dashboard, explore, components, …). Each PR gets the typed-dispatch swap and the v8 bump applied to that slice's typecheck closure.
3. **Final PR**: Bump `react-redux` in `package.json`, remove `@types/react-redux` from the three places it's declared (it's bundled with v8), drop any leftover `Dispatch` assertions, verify full project typechecks.

Alternative: a single mega-PR that does all of it at once. Higher review burden, harder to revert, blocks other work — generally not recommended unless someone wants to drive it as a focused effort.

## Files declaring `@types/react-redux` (to remove on bump)

- `superset-frontend/package.json`
- `superset-frontend/plugins/plugin-chart-echarts/package.json`
- `superset-frontend/plugins/plugin-chart-cartodiagram/package.json`

## Why this isn't blocking #39890

`useDeferredValue` works fine under react-redux v7. The "tearing risk" mitigated by v8's `useSyncExternalStore` only manifests with `useTransition`, so `useTransition` adoption (parent issue's tier-2 item) is what actually depends on this. We can ship `useDeferredValue` migrations against v7 in the meantime.

Contributor guide

Open the contributing guide

Research direction

Start with the existing store exports in src/views/store/index.ts and the three package.json files declaring @types/react-redux. Define and verify the typed hooks before migrating feature areas such as SqlLab, dashboard, and explore. Done means the dependency bump, call-site migrations, cleanup of obsolete assertions and types, and full project typecheck are complete.

Written by the indexing model from the issue text.

Assessment

Tech stack
react, typescript
Domain
frontend
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Quiet
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.