From 65d66dca8c0e2796e9b9d8bc24c741f75e9bcc52 Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Thu, 20 Feb 2020 15:50:26 +0100 Subject: [PATCH 1/2] remove `any` where applicable --- src/createAction.ts | 17 +++++-- src/entities/models.ts | 56 +++++++--------------- src/entities/sorted_state_adapter.test.ts | 2 +- src/entities/sorted_state_adapter.ts | 46 +++++++----------- src/entities/state_adapter.ts | 19 ++++---- src/entities/unsorted_state_adapter.ts | 57 +++++++++-------------- 6 files changed, 79 insertions(+), 118 deletions(-) diff --git a/src/createAction.ts b/src/createAction.ts index 14fda47545..0d6bdaafe2 100644 --- a/src/createAction.ts +++ b/src/createAction.ts @@ -296,11 +296,18 @@ export function createAction(type: string, prepareAction?: Function): any { return actionCreator } -export function isFSA< - Payload = undefined, - Type extends string = string, - Meta = undefined ->(action: any): action is PayloadAction { +/** + * The previous typings implied some assertion for the types of Payload/Error/Meta, which was just not the case. + * I'd suggest we just do something like this - assert the shape of a FSA, but make no assumptions about the contents. + */ +export function isFSA( + action: unknown +): action is { + type: string + payload?: unknown + error?: unknown + meta?: unknown +} { return ( isPlainObject(action) && typeof (action as any).type === 'string' && diff --git a/src/entities/models.ts b/src/entities/models.ts index a010dd166a..02b4655504 100644 --- a/src/entities/models.ts +++ b/src/entities/models.ts @@ -24,9 +24,13 @@ export interface DictionaryNum { /** * @alpha + * why was this a class, not an interface? */ export abstract class Dictionary implements DictionaryNum { - [id: string]: T | undefined + // I get why you're adding "undefined" here, but there is not one check for it actually not being undefined down the line + // on the contrary, some casts to `any` were necessary because `Object.entities(dictionary).sort()` were not possible any more + // due to the `sort` assuming parameters of T. + [id: string]: T } /** @@ -39,11 +43,6 @@ export type Update = { id: EntityId; changes: Partial } */ export type EntityMap = (entity: T) => T -/** - * @alpha - */ -export type TypeOrPayloadAction = T | PayloadAction - /** * @alpha */ @@ -58,71 +57,48 @@ export interface EntityDefinition { } export interface EntityStateAdapter { - addOne>(state: S, entity: TypeOrPayloadAction): S + addOne>(state: S, entity: T): S addOne>(state: S, action: PayloadAction): S - addMany>( - state: S, - entities: TypeOrPayloadAction - ): S + addMany>(state: S, entities: T[]): S addMany>(state: S, entities: PayloadAction): S - setAll>( - state: S, - entities: TypeOrPayloadAction - ): S + setAll>(state: S, entities: T[]): S setAll>(state: S, entities: PayloadAction): S - removeOne>( - state: S, - key: TypeOrPayloadAction - ): S + removeOne>(state: S, key: EntityId): S removeOne>(state: S, key: PayloadAction): S + removeMany>(state: S, keys: EntityId[]): S removeMany>( state: S, - keys: TypeOrPayloadAction + keys: PayloadAction ): S removeAll>(state: S): S - updateOne>( - state: S, - update: TypeOrPayloadAction> - ): S + updateOne>(state: S, update: Update): S updateOne>( state: S, update: PayloadAction> ): S - updateMany>( - state: S, - updates: TypeOrPayloadAction[]> - ): S + updateMany>(state: S, updates: Update[]): S updateMany>( state: S, updates: PayloadAction[]> ): S - upsertOne>( - state: S, - entity: TypeOrPayloadAction - ): S + upsertOne>(state: S, entity: T): S upsertOne>(state: S, entity: PayloadAction): S - upsertMany>( - state: S, - entities: TypeOrPayloadAction - ): S + upsertMany>(state: S, entities: T[]): S upsertMany>( state: S, entities: PayloadAction ): S - map>( - state: S, - map: TypeOrPayloadAction> - ): S + map>(state: S, map: EntityMap): S map>(state: S, map: PayloadAction>): S } diff --git a/src/entities/sorted_state_adapter.test.ts b/src/entities/sorted_state_adapter.test.ts index 661b033525..72e243ef97 100644 --- a/src/entities/sorted_state_adapter.test.ts +++ b/src/entities/sorted_state_adapter.test.ts @@ -1,4 +1,4 @@ -import { EntityStateAdapter, EntityState, Update } from './models' +import { EntityStateAdapter, EntityState } from './models' import { createEntityAdapter } from './create_adapter' import { createAction } from '../createAction' import { diff --git a/src/entities/sorted_state_adapter.ts b/src/entities/sorted_state_adapter.ts index 0d8c68d567..b2887caccc 100644 --- a/src/entities/sorted_state_adapter.ts +++ b/src/entities/sorted_state_adapter.ts @@ -4,7 +4,8 @@ import { Comparer, EntityStateAdapter, Update, - EntityMap + EntityMap, + EntityId } from './models' import { createStateOperator } from './state_adapter' import { createUnsortedStateAdapter } from './unsorted_state_adapter' @@ -13,21 +14,18 @@ import { selectIdValue } from './utils' export function createSortedStateAdapter( selectId: IdSelector, sort: Comparer -): EntityStateAdapter -export function createSortedStateAdapter(selectId: any, sort: any): any { +): EntityStateAdapter { type R = EntityState const { removeOne, removeMany, removeAll } = createUnsortedStateAdapter( selectId ) - function addOneMutably(entity: T, state: R): void - function addOneMutably(entity: any, state: any): void { + function addOneMutably(entity: T, state: R): void { return addManyMutably([entity], state) } - function addManyMutably(newModels: T[], state: R): void - function addManyMutably(newModels: any[], state: any): void { + function addManyMutably(newModels: T[], state: R): void { const models = newModels.filter( model => !(selectIdValue(model, selectId) in state.entities) ) @@ -37,21 +35,18 @@ export function createSortedStateAdapter(selectId: any, sort: any): any { } } - function setAllMutably(models: T[], state: R): void - function setAllMutably(models: any[], state: any): void { + function setAllMutably(models: T[], state: R): void { state.entities = {} state.ids = [] addManyMutably(models, state) } - function updateOneMutably(update: Update, state: R): void - function updateOneMutably(update: any, state: any): void { + function updateOneMutably(update: Update, state: R): void { return updateManyMutably([update], state) } - function takeUpdatedModel(models: T[], update: Update, state: R): boolean - function takeUpdatedModel(models: any[], update: any, state: any): boolean { + function takeUpdatedModel(models: T[], update: Update, state: R): boolean { if (!(update.id in state.entities)) { return false } @@ -67,8 +62,7 @@ export function createSortedStateAdapter(selectId: any, sort: any): any { return newKey !== update.id } - function updateManyMutably(updates: Update[], state: R): void - function updateManyMutably(updates: any[], state: any): void { + function updateManyMutably(updates: Update[], state: R): void { const models: T[] = [] updates.forEach(update => takeUpdatedModel(models, update, state)) @@ -78,11 +72,10 @@ export function createSortedStateAdapter(selectId: any, sort: any): any { } } - function mapMutably(map: EntityMap, state: R): void - function mapMutably(updatesOrMap: any, state: any): void { + function mapMutably(updatesOrMap: EntityMap, state: R): void { const updates: Update[] = state.ids.reduce( - (changes: any[], id: string | number) => { - const change = updatesOrMap(state.entities[id]) + (changes: Update[], id: EntityId) => { + const change = updatesOrMap(state.entities[id]!) if (change !== state.entities[id]) { changes.push({ id, changes: change }) } @@ -94,15 +87,13 @@ export function createSortedStateAdapter(selectId: any, sort: any): any { updateManyMutably(updates, state) } - function upsertOneMutably(entity: T, state: R): void - function upsertOneMutably(entity: any, state: any): void { + function upsertOneMutably(entity: T, state: R): void { return upsertManyMutably([entity], state) } - function upsertManyMutably(entities: T[], state: R): void - function upsertManyMutably(entities: any[], state: any): void { - const added: any[] = [] - const updated: any[] = [] + function upsertManyMutably(entities: T[], state: R): void { + const added: T[] = [] + const updated: Update[] = [] for (const entity of entities) { const id = selectIdValue(entity, selectId) @@ -117,7 +108,7 @@ export function createSortedStateAdapter(selectId: any, sort: any): any { addManyMutably(added, state) } - function areArraysEqual(a: any[], b: any[]) { + function areArraysEqual(a: unknown[], b: unknown[]) { if (a.length !== b.length) { return false } @@ -131,8 +122,7 @@ export function createSortedStateAdapter(selectId: any, sort: any): any { return true } - function merge(models: T[], state: R): void - function merge(models: any[], state: any): void { + function merge(models: T[], state: R): void { models.sort(sort) // Insert/overwrite all new/updated diff --git a/src/entities/state_adapter.ts b/src/entities/state_adapter.ts index d5c44fb894..a00089175c 100644 --- a/src/entities/state_adapter.ts +++ b/src/entities/state_adapter.ts @@ -1,19 +1,22 @@ -import createNextState, { Draft, isDraft } from 'immer' +import createNextState, { isDraft } from 'immer' import { EntityState } from './models' import { PayloadAction, isFSA } from '../createAction' export function createStateOperator( mutator: (arg: R, state: EntityState) => void -): EntityState -export function createStateOperator( - mutator: (arg: any, state: any) => void -): any { +) { return function operation>( - state: any, + state: S, arg: R | PayloadAction ): S { - const runMutator = (draft: Draft>) => { - if (isFSA(arg)) { + function isPayloadActionArgument( + arg: R | PayloadAction + ): arg is PayloadAction { + return isFSA(arg) + } + + const runMutator = (draft: EntityState) => { + if (isPayloadActionArgument(arg)) { mutator(arg.payload, draft) } else { mutator(arg, draft) diff --git a/src/entities/unsorted_state_adapter.ts b/src/entities/unsorted_state_adapter.ts index 1ab0cfd66c..5791abead7 100644 --- a/src/entities/unsorted_state_adapter.ts +++ b/src/entities/unsorted_state_adapter.ts @@ -3,19 +3,18 @@ import { EntityStateAdapter, IdSelector, Update, - EntityMap + EntityMap, + EntityId } from './models' import { createStateOperator } from './state_adapter' import { selectIdValue } from './utils' export function createUnsortedStateAdapter( selectId: IdSelector -): EntityStateAdapter -export function createUnsortedStateAdapter(selectId: IdSelector): any { +): EntityStateAdapter { type R = EntityState - function addOneMutably(entity: T, state: R): void - function addOneMutably(entity: any, state: any): void { + function addOneMutably(entity: T, state: EntityState): void { const key = selectIdValue(entity, selectId) if (key in state.entities) { @@ -26,27 +25,24 @@ export function createUnsortedStateAdapter(selectId: IdSelector): any { state.entities[key] = entity } - function addManyMutably(entities: T[], state: R): void - function addManyMutably(entities: any[], state: any): void { + function addManyMutably(entities: T[], state: R): void { for (const entity of entities) { addOneMutably(entity, state) } } - function setAllMutably(entities: T[], state: R): void - function setAllMutably(entities: any[], state: any): void { + function setAllMutably(entities: T[], state: R): void { state.ids = [] state.entities = {} addManyMutably(entities, state) } - function removeOneMutably(key: T, state: R): void - function removeOneMutably(key: any, state: any): void { + function removeOneMutably(key: EntityId, state: R): void { return removeManyMutably([key], state) } - function removeManyMutably(keys: any[], state: R): void { + function removeManyMutably(keys: EntityId[], state: R): void { let didMutate = false keys.forEach(key => { @@ -61,8 +57,7 @@ export function createUnsortedStateAdapter(selectId: IdSelector): any { } } - function removeAll(state: S): S - function removeAll(state: any): S { + function removeAll(state: S): S { return Object.assign({}, state, { ids: [], entities: {} @@ -70,14 +65,9 @@ export function createUnsortedStateAdapter(selectId: IdSelector): any { } function takeNewKey( - keys: { [id: string]: string }, + keys: { [id: string]: EntityId }, update: Update, state: R - ): void - function takeNewKey( - keys: { [id: string]: any }, - update: Update, - state: any ): boolean { const original = state.entities[update.id] const updated: T = Object.assign({}, original, update.changes) @@ -94,14 +84,12 @@ export function createUnsortedStateAdapter(selectId: IdSelector): any { return hasNewKey } - function updateOneMutably(update: Update, state: R): void - function updateOneMutably(update: any, state: any): void { + function updateOneMutably(update: Update, state: R): void { return updateManyMutably([update], state) } - function updateManyMutably(updates: Update[], state: R): void - function updateManyMutably(updates: any[], state: any): void { - const newKeys: { [id: string]: string } = {} + function updateManyMutably(updates: Update[], state: R): void { + const newKeys: { [id: string]: EntityId } = {} const updatesPerEntity: { [id: string]: Update } = {} @@ -127,16 +115,15 @@ export function createUnsortedStateAdapter(selectId: IdSelector): any { updates.filter(update => takeNewKey(newKeys, update, state)).length > 0 if (didMutateIds) { - state.ids = state.ids.map((id: any) => newKeys[id] || id) + state.ids = state.ids.map(id => newKeys[id] || id) } } } - function mapMutably(map: EntityMap, state: R): void - function mapMutably(map: any, state: any): void { + function mapMutably(map: EntityMap, state: R): void { const changes: Update[] = state.ids.reduce( - (changes: any[], id: string | number) => { - const change = map(state.entities[id]) + (changes: Update[], id: EntityId) => { + const change = map(state.entities[id]!) if (change !== state.entities[id]) { changes.push({ id, changes: change }) } @@ -149,15 +136,13 @@ export function createUnsortedStateAdapter(selectId: IdSelector): any { return updateManyMutably(updates, state) } - function upsertOneMutably(entity: T, state: R): void - function upsertOneMutably(entity: any, state: any): void { + function upsertOneMutably(entity: T, state: R): void { return upsertManyMutably([entity], state) } - function upsertManyMutably(entities: T[], state: R): void - function upsertManyMutably(entities: any[], state: any): void { - const added: any[] = [] - const updated: any[] = [] + function upsertManyMutably(entities: T[], state: R): void { + const added: T[] = [] + const updated: Update[] = [] for (const entity of entities) { const id = selectIdValue(entity, selectId) From e1093480216dd641e15a0f2822ffac164f44b2b2 Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Fri, 21 Feb 2020 17:04:03 +0100 Subject: [PATCH 2/2] re-add `| undefined`, remove review comments --- etc/redux-toolkit.api.md | 2 +- src/createAction.ts | 4 ---- src/entities/models.ts | 8 ++------ src/entities/sorted_state_adapter.ts | 2 +- 4 files changed, 4 insertions(+), 12 deletions(-) diff --git a/etc/redux-toolkit.api.md b/etc/redux-toolkit.api.md index 10cae823a8..f90952e015 100644 --- a/etc/redux-toolkit.api.md +++ b/etc/redux-toolkit.api.md @@ -159,7 +159,7 @@ export interface CreateSliceOptions implements DictionaryNum { +export interface Dictionary extends DictionaryNum { // (undocumented) [id: string]: T | undefined; } diff --git a/src/createAction.ts b/src/createAction.ts index 0d6bdaafe2..2dea13b369 100644 --- a/src/createAction.ts +++ b/src/createAction.ts @@ -296,10 +296,6 @@ export function createAction(type: string, prepareAction?: Function): any { return actionCreator } -/** - * The previous typings implied some assertion for the types of Payload/Error/Meta, which was just not the case. - * I'd suggest we just do something like this - assert the shape of a FSA, but make no assumptions about the contents. - */ export function isFSA( action: unknown ): action is { diff --git a/src/entities/models.ts b/src/entities/models.ts index 02b4655504..9f7b217d4f 100644 --- a/src/entities/models.ts +++ b/src/entities/models.ts @@ -24,13 +24,9 @@ export interface DictionaryNum { /** * @alpha - * why was this a class, not an interface? */ -export abstract class Dictionary implements DictionaryNum { - // I get why you're adding "undefined" here, but there is not one check for it actually not being undefined down the line - // on the contrary, some casts to `any` were necessary because `Object.entities(dictionary).sort()` were not possible any more - // due to the `sort` assuming parameters of T. - [id: string]: T +export interface Dictionary extends DictionaryNum { + [id: string]: T | undefined } /** diff --git a/src/entities/sorted_state_adapter.ts b/src/entities/sorted_state_adapter.ts index b2887caccc..79e6e03f3b 100644 --- a/src/entities/sorted_state_adapter.ts +++ b/src/entities/sorted_state_adapter.ts @@ -130,7 +130,7 @@ export function createSortedStateAdapter( state.entities[selectId(model)] = model }) - const allEntities = Object.values(state.entities) + const allEntities = Object.values(state.entities) as T[] allEntities.sort(sort) const newSortedIds = allEntities.map(selectId)