Skip to content
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.

Sign up for GitHub

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

remove any where applicable #377

Merged
merged 2 commits into from
Feb 21, 2020
Merged

remove any where applicable #377

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
65d66dc
remove `any` where applicable
Feb 20, 2020
e109348
re-add `| undefined`, remove review comments
Feb 21, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion etc/redux-toolkit.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -159,7 +159,7 @@ export interface CreateSliceOptions<State = any, CR extends SliceCaseReducers<St
}

// @alpha (undocumented)
export abstract class Dictionary<T> implements DictionaryNum<T> {
export interface Dictionary<T> extends DictionaryNum<T> {
// (undocumented)
[id: string]: T | undefined;
}
Expand Down
13 changes: 8 additions & 5 deletions src/createAction.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -296,11 +296,14 @@ 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<Payload, Type, Meta> {
export function isFSA(
phryneas marked this conversation as resolved.
Show resolved Hide resolved
action: unknown
): action is {
type: string
payload?: unknown
error?: unknown
meta?: unknown
} {
return (
isPlainObject(action) &&
typeof (action as any).type === 'string' &&
Expand Down
52 changes: 12 additions & 40 deletions src/entities/models.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ export interface DictionaryNum<T> {
/**
* @alpha
*/
export abstract class Dictionary<T> implements DictionaryNum<T> {
export interface Dictionary<T> extends DictionaryNum<T> {
[id: string]: T | undefined
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FWIW, this definition came straight from @ngrx/entity.

I think the intent here is more that the user-level code understands that booksState.entities["doesNotExist"] returns undefined, vs a Record<string, Book> where it doesn't.

So, yeah, the internal casts are a bit weird, but it seems like an okay tradeoff to get the right behavior at the user level?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah. That's the lifelong discussion in the TS issues if an index signature should actually be undefined per default and it comes down to "if you really want to signal this, add the | undefined.
I guess in this place it makes sense, so I'll re-add it.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

And yeah, I never understood why this was an abstract class to begin with either

}

Expand All @@ -39,11 +39,6 @@ export type Update<T> = { id: EntityId; changes: Partial<T> }
*/
export type EntityMap<T> = (entity: T) => T

/**
* @alpha
*/
export type TypeOrPayloadAction<T> = T | PayloadAction<T>

/**
* @alpha
*/
Expand All @@ -58,71 +53,48 @@ export interface EntityDefinition<T> {
}

export interface EntityStateAdapter<T> {
addOne<S extends EntityState<T>>(state: S, entity: TypeOrPayloadAction<T>): S
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the net benefit of dropping TypeOrPayloadAction here?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You had the function overload only taking the PayloadAction there anyways, so it's either remove that or cut this back to prevent duplication.

Since I remembered you mentioned that you had some problem with this TypeOrPayloadAction and having two overloads with different arguments somehow looked "cleaner" to me anyways, I decided to go the "two overload" way.

addOne<S extends EntityState<T>>(state: S, entity: T): S
addOne<S extends EntityState<T>>(state: S, action: PayloadAction<T>): S

addMany<S extends EntityState<T>>(
state: S,
entities: TypeOrPayloadAction<T[]>
): S
addMany<S extends EntityState<T>>(state: S, entities: T[]): S
addMany<S extends EntityState<T>>(state: S, entities: PayloadAction<T[]>): S

setAll<S extends EntityState<T>>(
state: S,
entities: TypeOrPayloadAction<T[]>
): S
setAll<S extends EntityState<T>>(state: S, entities: T[]): S
setAll<S extends EntityState<T>>(state: S, entities: PayloadAction<T[]>): S

removeOne<S extends EntityState<T>>(
state: S,
key: TypeOrPayloadAction<EntityId>
): S
removeOne<S extends EntityState<T>>(state: S, key: EntityId): S
removeOne<S extends EntityState<T>>(state: S, key: PayloadAction<EntityId>): S

removeMany<S extends EntityState<T>>(state: S, keys: EntityId[]): S
removeMany<S extends EntityState<T>>(
state: S,
keys: TypeOrPayloadAction<EntityId[]>
keys: PayloadAction<EntityId[]>
): S

removeAll<S extends EntityState<T>>(state: S): S

updateOne<S extends EntityState<T>>(
state: S,
update: TypeOrPayloadAction<Update<T>>
): S
updateOne<S extends EntityState<T>>(state: S, update: Update<T>): S
updateOne<S extends EntityState<T>>(
state: S,
update: PayloadAction<Update<T>>
): S

updateMany<S extends EntityState<T>>(
state: S,
updates: TypeOrPayloadAction<Update<T>[]>
): S
updateMany<S extends EntityState<T>>(state: S, updates: Update<T>[]): S
updateMany<S extends EntityState<T>>(
state: S,
updates: PayloadAction<Update<T>[]>
): S

upsertOne<S extends EntityState<T>>(
state: S,
entity: TypeOrPayloadAction<T>
): S
upsertOne<S extends EntityState<T>>(state: S, entity: T): S
upsertOne<S extends EntityState<T>>(state: S, entity: PayloadAction<T>): S

upsertMany<S extends EntityState<T>>(
state: S,
entities: TypeOrPayloadAction<T[]>
): S
upsertMany<S extends EntityState<T>>(state: S, entities: T[]): S
upsertMany<S extends EntityState<T>>(
state: S,
entities: PayloadAction<T[]>
): S

map<S extends EntityState<T>>(
state: S,
map: TypeOrPayloadAction<EntityMap<T>>
): S
map<S extends EntityState<T>>(state: S, map: EntityMap<T>): S
map<S extends EntityState<T>>(state: S, map: PayloadAction<EntityMap<T>>): S
}

Expand Down
2 changes: 1 addition & 1 deletion src/entities/sorted_state_adapter.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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 {
Expand Down
48 changes: 19 additions & 29 deletions src/entities/sorted_state_adapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ import {
Comparer,
EntityStateAdapter,
Update,
EntityMap
EntityMap,
EntityId
} from './models'
import { createStateOperator } from './state_adapter'
import { createUnsortedStateAdapter } from './unsorted_state_adapter'
Expand All @@ -13,21 +14,18 @@ import { selectIdValue } from './utils'
export function createSortedStateAdapter<T>(
selectId: IdSelector<T>,
sort: Comparer<T>
): EntityStateAdapter<T>
export function createSortedStateAdapter<T>(selectId: any, sort: any): any {
): EntityStateAdapter<T> {
type R = EntityState<T>

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)
)
Expand All @@ -37,21 +35,18 @@ export function createSortedStateAdapter<T>(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<T>, state: R): void
function updateOneMutably(update: any, state: any): void {
function updateOneMutably(update: Update<T>, state: R): void {
return updateManyMutably([update], state)
}

function takeUpdatedModel(models: T[], update: Update<T>, state: R): boolean
function takeUpdatedModel(models: any[], update: any, state: any): boolean {
function takeUpdatedModel(models: T[], update: Update<T>, state: R): boolean {
if (!(update.id in state.entities)) {
return false
}
Expand All @@ -67,8 +62,7 @@ export function createSortedStateAdapter<T>(selectId: any, sort: any): any {
return newKey !== update.id
}

function updateManyMutably(updates: Update<T>[], state: R): void
function updateManyMutably(updates: any[], state: any): void {
function updateManyMutably(updates: Update<T>[], state: R): void {
const models: T[] = []

updates.forEach(update => takeUpdatedModel(models, update, state))
Expand All @@ -78,11 +72,10 @@ export function createSortedStateAdapter<T>(selectId: any, sort: any): any {
}
}

function mapMutably(map: EntityMap<T>, state: R): void
function mapMutably(updatesOrMap: any, state: any): void {
function mapMutably(updatesOrMap: EntityMap<T>, state: R): void {
const updates: Update<T>[] = state.ids.reduce(
(changes: any[], id: string | number) => {
const change = updatesOrMap(state.entities[id])
(changes: Update<T>[], id: EntityId) => {
const change = updatesOrMap(state.entities[id]!)
if (change !== state.entities[id]) {
changes.push({ id, changes: change })
}
Expand All @@ -94,15 +87,13 @@ export function createSortedStateAdapter<T>(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<T>[] = []

for (const entity of entities) {
const id = selectIdValue(entity, selectId)
Expand All @@ -117,7 +108,7 @@ export function createSortedStateAdapter<T>(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
}
Expand All @@ -131,16 +122,15 @@ export function createSortedStateAdapter<T>(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
models.forEach(model => {
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)
Expand Down
19 changes: 11 additions & 8 deletions src/entities/state_adapter.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -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<V, R>(
mutator: (arg: R, state: EntityState<V>) => void
): EntityState<V>
export function createStateOperator<V, R>(
mutator: (arg: any, state: any) => void
): any {
) {
return function operation<S extends EntityState<V>>(
state: any,
state: S,
arg: R | PayloadAction<R>
): S {
const runMutator = (draft: Draft<EntityState<V>>) => {
if (isFSA(arg)) {
function isPayloadActionArgument(
arg: R | PayloadAction<R>
): arg is PayloadAction<R> {
return isFSA(arg)
}

const runMutator = (draft: EntityState<V>) => {
if (isPayloadActionArgument(arg)) {
mutator(arg.payload, draft)
} else {
mutator(arg, draft)
Expand Down
Loading