You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "rusackas (via GitHub)" <gi...@apache.org> on 2026/05/07 03:31:17 UTC
[I] Migrate to react-redux v8 with a typed AppDispatch / useAppDispatch [superset]
rusackas opened a new issue, #39927:
URL: https://github.com/apache/superset/issues/39927
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<X>' is not assignable to parameter of type 'AnyAction'
error TS2344: Type '(dispatch: any) => Promise<…>' does not satisfy the constraint 'Dispatch<AnyAction>'
```
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<RootState, unknown, AnyAction>;
export const useAppDispatch: () => AppDispatch = useDispatch;
export const useAppSelector: TypedUseSelectorHook<RootState> = 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<AnyAction>` 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] Migrate to react-redux v8 with a typed AppDispatch / useAppDispatch [superset]
Posted by "venkatamandavilli-code (via GitHub)" <gi...@apache.org>.
venkatamandavilli-code commented on issue #39927:
URL: https://github.com/apache/superset/issues/39927#issuecomment-4471332681
Thanks for reporting this issue and sharing the details.
This looks like it may be related to how Superset is handling dashboard/chart state after the filter or refresh action runs. In some cases, the UI can briefly keep older cached values if the chart refresh and state update are not fully synchronized.
It may be worth checking whether the issue still happens after a hard refresh, with cache disabled, or in a separate browser session. That could help confirm whether this is related to cached query results, frontend state refresh, or the filter initialization flow.
A small regression test around repeated filter changes and chart refresh behavior might also help prevent this from coming back in future releases.
Thanks again for documenting it clearly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [I] Migrate to react-redux v8 with a typed AppDispatch / useAppDispatch [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas closed issue #39927: Migrate to react-redux v8 with a typed AppDispatch / useAppDispatch
URL: https://github.com/apache/superset/issues/39927
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org