-
Notifications
You must be signed in to change notification settings - Fork 8.6k
Bump redux (4.2.0), reselect (4.1.6), @reduxjs/toolkit (1.7.2), redux-thunk (2.4.1) and immer (9.0.15) #138818
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
eaf5f74
a66d9e6
f28d554
d8b0efb
7ba5e8e
b534d5c
920ff82
82e8f9c
e1f5c21
03a2460
c09552a
c008042
41a2b8e
5188afa
6eb623d
1e04730
df8279b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -554,28 +554,9 @@ export const nodesAndEdgelines: (state: DataState) => ( | |
| processNodePositions: visibleProcessNodePositions, | ||
| connectingEdgeLineSegments, | ||
| }; | ||
| }, aaBBEqualityCheck); | ||
| }, aabbModel.isEqual); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. New reselect package correctly induces the type here, so the helper function below is no longer necessary. Functionality is the same. |
||
| }); | ||
|
|
||
| function isAABBType(value: unknown): value is AABB { | ||
| const castValue = value as AABB; | ||
| return castValue.maximum !== undefined && castValue.minimum !== undefined; | ||
| } | ||
|
|
||
| /** | ||
| * This is needed to avoid the TS error that is caused by using aabbModel.isEqual directly. Ideally we could | ||
| * just pass that function instead of having to check the type of the parameters. It might be worth doing a PR to | ||
| * the reselect library to correct the type. | ||
| */ | ||
| function aaBBEqualityCheck<T>(a: T, b: T, index: number): boolean { | ||
| if (isAABBType(a) && isAABBType(b)) { | ||
| return aabbModel.isEqual(a, b); | ||
| } else { | ||
| // this is equivalent to the default equality check for defaultMemoize | ||
| return a === b; | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * If there is a pending request that's for a entity ID that doesn't matche the `entityID`, then we should cancel it. | ||
| */ | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -9,7 +9,13 @@ import { createSelector, defaultMemoize } from 'reselect'; | |
| import * as cameraSelectors from './camera/selectors'; | ||
| import * as dataSelectors from './data/selectors'; | ||
| import * as uiSelectors from './ui/selectors'; | ||
| import type { ResolverState, IsometricTaxiLayout, DataState } from '../types'; | ||
| import type { | ||
| ResolverState, | ||
| IsometricTaxiLayout, | ||
| DataState, | ||
| VisibleEntites, | ||
| NodeData, | ||
| } from '../types'; | ||
| import type { EventStats } from '../../../common/endpoint/types'; | ||
| import * as nodeModel from '../../../common/endpoint/models/node'; | ||
|
|
||
|
|
@@ -211,14 +217,9 @@ export const statsTotalForNode = composeSelectors( | |
| export const visibleNodesAndEdgeLines = createSelector( | ||
| nodesAndEdgelines, | ||
| boundingBox, | ||
| function ( | ||
| /* eslint-disable @typescript-eslint/no-shadow */ | ||
| nodesAndEdgelines, | ||
| boundingBox | ||
| /* eslint-enable @typescript-eslint/no-shadow */ | ||
| ) { | ||
| function (nodesAndEdgelinesFn, boundingBoxFn) { | ||
| // `boundingBox` and `nodesAndEdgelines` are each memoized. | ||
| return (time: number) => nodesAndEdgelines(boundingBox(time)); | ||
| return (time: number) => nodesAndEdgelinesFn(boundingBoxFn(time)); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TypeScript did not like the variable name shadowing here, fixing that fixed a type issue below. |
||
| } | ||
| ); | ||
|
|
||
|
|
@@ -240,12 +241,13 @@ export const originID: (state: ResolverState) => string | undefined = composeSel | |
| * Takes a nodeID (aka entity_id) and returns the node ID of the node that aria should 'flowto' or null | ||
| * If the node has a flowto candidate that is currently visible, that will be returned, otherwise null. | ||
| */ | ||
| export const ariaFlowtoNodeID: ( | ||
| state: ResolverState | ||
| ) => (time: number) => (nodeID: string) => string | null = createSelector( | ||
| export const ariaFlowtoNodeID = createSelector( | ||
| visibleNodesAndEdgeLines, | ||
| composeSelectors(dataStateSelector, dataSelectors.ariaFlowtoCandidate), | ||
| (visibleNodesAndEdgeLinesAtTime, ariaFlowtoCandidate) => { | ||
| function ( | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Couldn't get this to work with an arrow function, TypeScript stopped complaining as soon as I turned this into an explicit |
||
| visibleNodesAndEdgeLinesAtTime: (time: number) => VisibleEntites, | ||
| ariaFlowtoCandidate: (nodeId: string) => string | null | ||
| ) { | ||
| return defaultMemoize((time: number) => { | ||
| // get the visible nodes at `time` | ||
| const { processNodePositions } = visibleNodesAndEdgeLinesAtTime(time); | ||
|
|
@@ -361,7 +363,10 @@ export const newIDsToRequest: (state: ResolverState) => (time: number) => Set<st | |
| createSelector( | ||
| composeSelectors(dataStateSelector, (dataState: DataState) => dataState.nodeData), | ||
| visibleNodesAndEdgeLines, | ||
| function (nodeData, visibleNodesAndEdgeLinesAtTime) { | ||
| function ( | ||
| nodeData: Map<string, NodeData> | undefined, | ||
| visibleNodesAndEdgeLinesAtTime: (time: number) => VisibleEntites | ||
| ) { | ||
| return defaultMemoize((time: number) => { | ||
| const { processNodePositions: nodesInView } = visibleNodesAndEdgeLinesAtTime(time); | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,16 +7,30 @@ | |
|
|
||
| import { combineReducers } from '@reduxjs/toolkit'; | ||
|
|
||
| import { syntheticsMonitorReducer } from './monitor_summary/synthetics_montior_reducer'; | ||
| import { monitorStatusReducer } from './monitor_summary'; | ||
| import { uiReducer } from './ui'; | ||
| import { indexStatusReducer } from './index_status'; | ||
| import { syntheticsEnablementReducer } from './synthetics_enablement'; | ||
| import { monitorListReducer } from './monitor_list'; | ||
| import { serviceLocationsReducer } from './service_locations'; | ||
| import { monitorOverviewReducer } from './overview'; | ||
| import { | ||
| syntheticsMonitorReducer, | ||
| SyntheticsMonitorState, | ||
| } from './monitor_summary/synthetics_montior_reducer'; | ||
| import { monitorStatusReducer, MonitorSummaryState } from './monitor_summary'; | ||
| import { uiReducer, UiState } from './ui'; | ||
| import { indexStatusReducer, IndexStatusState } from './index_status'; | ||
| import { syntheticsEnablementReducer, SyntheticsEnablementState } from './synthetics_enablement'; | ||
| import { monitorListReducer, MonitorListState } from './monitor_list'; | ||
| import { serviceLocationsReducer, ServiceLocationsState } from './service_locations'; | ||
| import { monitorOverviewReducer, MonitorOverviewState } from './overview'; | ||
|
|
||
| export const rootReducer = combineReducers({ | ||
| export interface SyntheticsAppState { | ||
| ui: UiState; | ||
| indexStatus: IndexStatusState; | ||
| syntheticsEnablement: SyntheticsEnablementState; | ||
| monitorList: MonitorListState; | ||
| serviceLocations: ServiceLocationsState; | ||
| monitorStatus: MonitorSummaryState; | ||
| syntheticsMonitor: SyntheticsMonitorState; | ||
| overview: MonitorOverviewState; | ||
| } | ||
|
Comment on lines
22
to
31
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @sphilipse I suppose this explicit typing was needed because redux toolkit's internal types aren't stable between versions?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, more recent versions led to a bunch of type errors. Explicit typing fixed it.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @awahab07, can you give this PR an approval or are there any changes you want to see? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi, I'm a Redux Toolkit maintainer. I'm curious - what type errors did you see with this update? We actually specifically recommend inferring the root state from the result of |
||
|
|
||
| export const rootReducer = combineReducers<SyntheticsAppState>({ | ||
| ui: uiReducer, | ||
| indexStatus: indexStatusReducer, | ||
| syntheticsEnablement: syntheticsEnablementReducer, | ||
|
|
@@ -26,5 +40,3 @@ export const rootReducer = combineReducers({ | |
| syntheticsMonitor: syntheticsMonitorReducer, | ||
| overview: monitorOverviewReducer, | ||
| }); | ||
|
|
||
| export type SyntheticsAppState = ReturnType<typeof rootReducer>; | ||
Uh oh!
There was an error while loading. Please reload this page.