diff --git a/libs/ngrx-toolkit/src/lib/with-entity-resources.spec.ts b/libs/ngrx-toolkit/src/lib/with-entity-resources.spec.ts index 131b2e0b..1886fa83 100644 --- a/libs/ngrx-toolkit/src/lib/with-entity-resources.spec.ts +++ b/libs/ngrx-toolkit/src/lib/with-entity-resources.spec.ts @@ -7,6 +7,7 @@ import { setAllEntities, } from '@ngrx/signals/entities'; import { withEntityResources } from './with-entity-resources'; +import { withPreviousValue } from './with-resource/tests/util/snapshot'; type Todo = { id: number; title: string; completed: boolean }; const wait = (ms = 0) => new Promise((r) => setTimeout(r, ms)); @@ -199,6 +200,99 @@ describe('withEntityResources', () => { ]); }); + it('does not support setAllEntities/addEntity/removeEntity for unnamed which are of type Resource', async () => { + const Store = signalStore( + { providedIn: 'root', protectedState: false }, + withEntityResources(() => + withPreviousValue( + resource({ + loader: () => Promise.resolve([] as Todo[]), + defaultValue: [], + }), + ), + ), + ); + const store = TestBed.inject(Store); + + await wait(); + + const invalidSetAll = () => + patchState( + store, + // @ts-expect-error Resources which are of type Resource should not support entity updaters + setAllEntities([ + { id: 1, title: 'A', completed: false }, + { id: 2, title: 'B', completed: true }, + ] as Todo[]), + ); + + const invalidAdd = () => + patchState( + store, + // @ts-expect-error Resources which are of type Resource should not support entity updaters + addEntity({ id: 3, title: 'C', completed: false } as Todo), + ); + + // @ts-expect-error Resources which are of type Resource should not support entity updaters + const invalidRemove = () => patchState(store, removeEntity(2)); + + expect(invalidSetAll).toBeDefined(); + expect(invalidAdd).toBeDefined(); + expect(invalidRemove).toBeDefined(); + expect(store.ids()).toEqual([]); + expect(store.entities()).toEqual([]); + }); + + it('does not support setAllEntities/addEntity/removeEntity for named which are of type Resource', async () => { + const Store = signalStore( + { providedIn: 'root', protectedState: false }, + withEntityResources(() => ({ + todos: withPreviousValue( + resource({ + loader: () => Promise.resolve([] as Todo[]), + defaultValue: [], + }), + ), + })), + ); + + const store = TestBed.inject(Store); + + await wait(); + + const invalidSetAll = () => + patchState( + store, + // @ts-expect-error Resources which are of type Resource should not support entity updaters + setAllEntities( + [ + { id: 1, title: 'A', completed: false }, + { id: 2, title: 'B', completed: true }, + ] as Todo[], + { collection: 'todos' }, + ), + ); + + const invalidAdd = () => + patchState( + store, + // @ts-expect-error Resources which are of type Resource should not support entity updaters + addEntity({ id: 3, title: 'C', completed: false } as Todo, { + collection: 'todos', + }), + ); + + const invalidRemove = () => + // @ts-expect-error Resources which are of type Resource should not support entity updaters + patchState(store, removeEntity(2, { collection: 'todos' })); + + expect(invalidSetAll).toBeDefined(); + expect(invalidAdd).toBeDefined(); + expect(invalidRemove).toBeDefined(); + expect(store.todosIds()).toEqual([]); + expect(store.todosEntities()).toEqual([]); + }); + it('supports setAllEntities/addEntity/removeEntity for named', async () => { const Store = signalStore( { providedIn: 'root', protectedState: false }, diff --git a/libs/ngrx-toolkit/src/lib/with-entity-resources.ts b/libs/ngrx-toolkit/src/lib/with-entity-resources.ts index f3afac0f..91fac185 100644 --- a/libs/ngrx-toolkit/src/lib/with-entity-resources.ts +++ b/libs/ngrx-toolkit/src/lib/with-entity-resources.ts @@ -1,4 +1,13 @@ -import { ResourceRef, Signal, isSignal, linkedSignal } from '@angular/core'; +import { + Resource, + ResourceRef, + ResourceSnapshot, + ResourceStatus, + Signal, + computed, + isSignal, + linkedSignal, +} from '@angular/core'; import { SignalStoreFeature, SignalStoreFeatureResult, @@ -6,6 +15,7 @@ import { signalStoreFeature, withComputed, withLinkedState, + withProps, } from '@ngrx/signals'; import { EntityId, @@ -17,7 +27,6 @@ import { import { NamedResourceResult, ResourceResult, - isResourceRef, withResource, } from './with-resource'; @@ -94,6 +103,15 @@ export function withEntityResources< resourceFactory: ( store: Input['props'] & Input['methods'] & StateSignals, ) => ResourceRef>, +): SignalStoreFeature>; + +export function withEntityResources< + Input extends SignalStoreFeatureResult, + Entity extends { id: EntityId }, +>( + resourceFactory: ( + store: Input['props'] & Input['methods'] & StateSignals, + ) => Resource>, ): SignalStoreFeature>; export function withEntityResources< @@ -111,7 +129,7 @@ export function withEntityResources< >( entityResourceFactory: ( store: Input['props'] & Input['methods'] & StateSignals, - ) => ResourceRef | EntityDictionary, + ) => Resource | EntityDictionary, ): SignalStoreFeature { return (store) => { const resourceOrDict = entityResourceFactory({ @@ -120,7 +138,9 @@ export function withEntityResources< ...store.methods, }); - if (isResourceRef(resourceOrDict)) { + if (isEntityResourceRef(resourceOrDict)) { + return createUnnamedEntityResourceRef(resourceOrDict)(store); + } else if (isEntityResource(resourceOrDict)) { return createUnnamedEntityResource(resourceOrDict)(store); } return createNamedEntityResources(resourceOrDict)(store); @@ -133,13 +153,29 @@ export function withEntityResources< * because {@link withResource} creates a Proxy around the resource value * to avoid the error throwing behavior of the Resource API. */ -function createUnnamedEntityResource( +function createUnnamedEntityResourceRef( resource: ResourceRef>, ) { return signalStoreFeature( withResource(() => resource), - withLinkedState(({ value }) => { - const { ids, entityMap } = createEntityDerivations(value); + withLinkedState((store) => { + // `value` exists in either the `props` or `state`, but `store` here only + // infers the `state` case, so we have to do this bleh casting + const propsOrStateStore = store as { + value: Signal>; + status: Signal; + error: Signal; + isLoading: Signal; + snapshot: Signal>>; + }; + const resourceValue = propsOrStateStore[ + `value` + ] as Signal; + if (!isSignal(resourceValue)) { + throw new Error(`Resource's value ${name}Value does not exist`); + } + + const { ids, entityMap } = createEntityDerivations(resourceValue); return { entityMap, @@ -152,6 +188,38 @@ function createUnnamedEntityResource( ); } +function createUnnamedEntityResource( + resource: Resource>, +) { + return signalStoreFeature( + withResource(() => resource), + withProps((store) => { + const propsStore = store as { + value: Signal>; + status: Signal; + error: Signal; + isLoading: Signal; + snapshot: Signal>>; + }; + + const resourceValue = propsStore.value as Signal; + if (!isSignal(resourceValue)) { + throw new Error(`Resource's value signal does not exist`); + } + + const { ids, entityMap } = createReadonlyEntityDerivations(resourceValue); + + return { + ids, + entityMap, + }; + }), + withComputed(({ ids, entityMap }) => ({ + entities: createComputedEntities(ids, entityMap), + })), + ); +} + /** * See {@link createUnnamedEntityResource} for why we cannot use the value of `resource` directly. */ @@ -160,23 +228,46 @@ function createNamedEntityResources( ) { const keys = Object.keys(dictionary); - const stateFactories = keys.map((name) => { - return (store: Record) => { - const resourceValue = store[ - `${name}Value` - ] as Signal; - if (!isSignal(resourceValue)) { - throw new Error(`Resource's value ${name}Value does not exist`); - } + const stateFactories = keys + .filter((name) => isEntityResourceRef(dictionary[name])) + .map((name) => { + return (store: Record) => { + const resourceValue = store[ + `${name}Value` + ] as Signal; + if (!isSignal(resourceValue)) { + throw new Error(`Resource's value ${name}Value does not exist`); + } - const { ids, entityMap } = createEntityDerivations(resourceValue); + const { ids, entityMap } = createEntityDerivations(resourceValue); - return { - [`${name}EntityMap`]: entityMap, - [`${name}Ids`]: ids, + return { + [`${name}EntityMap`]: entityMap, + [`${name}Ids`]: ids, + }; }; - }; - }); + }); + + const propsFactories = keys + .filter((name) => !isEntityResourceRef(dictionary[name])) + .map((name) => { + return (store: Record) => { + const resourceValue = store[ + `${name}Value` + ] as Signal; + if (!isSignal(resourceValue)) { + throw new Error(`Resource's value ${name}Value does not exist`); + } + + const { ids, entityMap } = + createReadonlyEntityDerivations(resourceValue); + + return { + [`${name}EntityMap`]: entityMap, + [`${name}Ids`]: ids, + }; + }; + }); const computedFactories = keys.map((name) => { return (store: Record) => { @@ -204,13 +295,28 @@ function createNamedEntityResources( withResource(() => dictionary), withLinkedState((store) => stateFactories.reduce( - (acc, factory) => ({ ...acc, ...factory(store) }), + (acc, factory) => ({ + ...acc, + ...factory(store), + }), + {}, + ), + ), + withProps((store) => + propsFactories.reduce( + (acc, factory) => ({ + ...acc, + ...factory(store), + }), {}, ), ), withComputed((store) => computedFactories.reduce( - (acc, factory) => ({ ...acc, ...factory(store) }), + (acc, factory) => ({ + ...acc, + ...factory(store), + }), {}, ), ), @@ -238,12 +344,37 @@ function createNamedEntityResources( * - For named resources we return `NamedResourceResult` intersected with * `NamedEntityState` and `NamedEntityProps` for each entry. */ -export type EntityResourceResult = { +declare const NON_PATCHABLE_ENTITY_RESOURCE_STATE: unique symbol; + +type NonPatchableEntityResourceStateMarker = { + [NON_PATCHABLE_ENTITY_RESOURCE_STATE]?: never; +}; + +type ReadonlyEntityProps = { + ids: Signal; + entityMap: Signal>; +}; + +type NamedReadonlyEntityProps = { + [Prop in `${Name}Ids`]: Signal; +} & { + [Prop in `${Name}EntityMap`]: Signal>; +}; + +export type EntityResourceRefResult = { state: ResourceResult['state'] & EntityState; props: ResourceResult['props'] & EntityProps; methods: ResourceResult['methods']; }; +export type EntityResourceResult = { + state: NonPatchableEntityResourceStateMarker; + props: ResourceResult['props'] & + EntityProps & + ReadonlyEntityProps; + methods: ResourceResult['methods']; +}; + // Generic helpers for inferring entity types and merging unions type ArrayElement = T extends readonly (infer E)[] | (infer E)[] ? E : never; @@ -262,20 +393,41 @@ type EntityResourceValue = Entity[] | (Entity[] | undefined); type TypedEntityResourceValue = E[] | (E[] | undefined); -export type EntityDictionary = Record>; +export type EntityDictionary = Record< + string, + ResourceRef | Resource +>; type MergeNamedEntityStates = MergeUnion< { [Prop in keyof T]: Prop extends string - ? InferEntityFromSignal extends infer E - ? E extends never - ? never - : NamedEntityState + ? T[Prop] extends ResourceRef + ? InferEntityFromSignal extends infer E + ? E extends never + ? never + : NamedEntityState + : never : never : never; }[keyof T] >; +type MergeNamedReadonlyEntityProps = MergeUnion< + { + [Prop in keyof T]: Prop extends string + ? T[Prop] extends ResourceRef + ? never + : InferEntityFromSignal extends infer E + ? E extends Entity + ? NamedReadonlyEntityProps + : E extends never + ? never + : never + : never + : never; + }[keyof T] +>; + type MergeNamedEntityProps = MergeUnion< { [Prop in keyof T]: Prop extends string @@ -289,8 +441,12 @@ type MergeNamedEntityProps = MergeUnion< >; export type NamedEntityResourceResult = { - state: NamedResourceResult['state'] & MergeNamedEntityStates; - props: NamedResourceResult['props'] & MergeNamedEntityProps; + state: NamedResourceResult['state'] & + MergeNamedEntityStates & + NonPatchableEntityResourceStateMarker; + props: NamedResourceResult['props'] & + MergeNamedEntityProps & + MergeNamedReadonlyEntityProps; methods: NamedResourceResult['methods']; }; @@ -341,6 +497,22 @@ function createEntityDerivations( return { ids, entityMap }; } +function createReadonlyEntityDerivations( + source: Signal>, +) { + const ids = computed(() => (source() ?? []).map((e) => e.id)); + + const entityMap = computed(() => { + const map = {} as Record; + for (const item of source() ?? []) { + map[item.id] = item as E; + } + return map; + }); + + return { ids, entityMap }; +} + function createComputedEntities( ids: Signal, entityMap: Signal>, @@ -349,3 +521,31 @@ function createComputedEntities( return ids().map((id) => entityMap()[id]); }; } + +export function isEntityResource( + value: unknown, +): value is Resource { + return ( + value !== null && + typeof value === 'object' && + 'value' in value && + isSignal(value.value) && + 'status' in value && + 'error' in value && + 'isLoading' in value && + 'snapshot' in value && + 'hasValue' in value + ); +} + +export function isEntityResourceRef( + value: unknown, +): value is ResourceRef { + return ( + isEntityResource(value) && + 'reload' in value && + 'set' in value && + 'update' in value && + 'asReadonly' in value + ); +} diff --git a/libs/ngrx-toolkit/src/lib/with-resource.spec.ts b/libs/ngrx-toolkit/src/lib/with-resource.spec.ts index 5958eae0..0a2f41f3 100644 --- a/libs/ngrx-toolkit/src/lib/with-resource.spec.ts +++ b/libs/ngrx-toolkit/src/lib/with-resource.spec.ts @@ -16,9 +16,14 @@ import { import { of } from 'rxjs'; import { Assert, AssertNot, IsEqual, Satisfies } from './test-utils/types'; import { ErrorHandling, mapToResource, withResource } from './with-resource'; +import { + stuffExtendedResourceReadable, + stuffExtendedResourceWritable, +} from './with-resource/tests/util/custom-extended-resources'; import { Address, venice, vienna } from './with-resource/tests/util/fixtures'; import { paramsForResourceTypes } from './with-resource/tests/util/params-for-resource-types'; import { setupUnnamedResource } from './with-resource/tests/util/setup-unnamed-resource'; +import { withPreviousValue } from './with-resource/tests/util/snapshot'; describe('withResource', () => { describe('standard tests', () => { @@ -263,6 +268,237 @@ describe('withResource', () => { }); }); + describe('extra properties checks', () => { + describe('extra properties go to `props` so they cannot be written to', () => { + it('for unnamed writables', async () => { + const Store = signalStore( + { providedIn: 'root', protectedState: false }, + withResource(() => stuffExtendedResourceWritable(() => 'a')), + ); + + const store = TestBed.inject(Store); + + await wait(); + + expect(store.stuff()).toBe('a stuff'); + + // @ts-expect-error - extra properties should not be patchable + patchState(store, { stuff: 'b stuff' }); + }); + it('for unnamed readables', async () => { + const Store = signalStore( + { providedIn: 'root', protectedState: false }, + withResource(() => stuffExtendedResourceReadable(() => 'a')), + ); + + const store = TestBed.inject(Store); + + await wait(); + + expect(store.stuff()).toBe('a stuff'); + + // @ts-expect-error - extra properties should not be patchable + patchState(store, { stuff: 'b stuff' }); + }); + it('for named readables', async () => { + const Store = signalStore( + { providedIn: 'root', protectedState: false }, + withResource(() => ({ + name: stuffExtendedResourceReadable(() => 'a'), + })), + ); + + const store = TestBed.inject(Store); + + await wait(); + + expect(store.nameStuff()).toBe('a stuff'); + + // @ts-expect-error - extra properties should not be patchable + patchState(store, { nameStuff: 'b stuff' }); + }); + it('for named writables', async () => { + const Store = signalStore( + { providedIn: 'root', protectedState: false }, + withResource(() => ({ + name: stuffExtendedResourceWritable(() => 'a'), + })), + ); + + const store = TestBed.inject(Store); + + await wait(); + + expect(store.nameStuff()).toBe('a stuff'); + + // @ts-expect-error - extra properties should not be patchable + patchState(store, { nameStuff: 'b stuff' }); + }); + }); + it('can call custom unnamed writable resource and extract custom extra signal properties', async () => { + const Store = signalStore( + { providedIn: 'root', protectedState: false }, + withResource(() => stuffExtendedResourceWritable(() => 'a')), + ); + const store = TestBed.inject(Store); + + await wait(); + + expect(store.value()).toBe('a'); + expect(store.stuff()).toBe('a stuff'); + + patchState(store, { value: 'b' }); + + expect(store.value()).toBe('b'); + expect(store.stuff()).toBe('b stuff'); + }); + it('can call custom unnamed readable resource and extract custom extra signal properties', async () => { + const Store = signalStore( + { providedIn: 'root', protectedState: false }, + withResource(() => stuffExtendedResourceReadable(() => 'a')), + ); + const store = TestBed.inject(Store); + + await wait(); + + expect(store.value()).toBe('a'); + expect(store.stuff()).toBe('a stuff'); + + // @ts-expect-error - readable resources should not have their value patchable + patchState(store, { value: 'b' }); + + expect(store.value()).toBe('a'); + expect(store.stuff()).toBe('a stuff'); + }); + it('can supply custom named resources (writable and unwritable) and extract custom extra signal properties', async () => { + const Store = signalStore( + { providedIn: 'root', protectedState: false }, + withResource(() => ({ + idWritable: stuffExtendedResourceWritable(() => 'a'), + idReadable: stuffExtendedResourceReadable(() => 'a'), + })), + ); + const store = TestBed.inject(Store); + + await wait(); + + expect(store.idWritableValue()).toBe('a'); + expect(store.idReadableValue()).toBe('a'); + expect(store.idWritableStuff()).toBe('a stuff'); + expect(store.idReadableStuff()).toBe('a stuff'); + + patchState(store, { idWritableValue: 'b' }); + // @ts-expect-error - readable resources should not have their value patchable + patchState(store, { idReadableValue: 'b' }); + + expect(store.idWritableValue()).toBe('b'); + expect(store.idReadableValue()).toBe('a'); + expect(store.idWritableStuff()).toBe('b stuff'); + expect(store.idReadableStuff()).toBe('a stuff'); + }); + + it('can supply custom unnamed (writable and unwritable) and custom named (writable and unwritable) and extract custom extra signal properties', async () => { + const UnnamedWritableAndNamedComboStore = signalStore( + { providedIn: 'root', protectedState: false }, + withResource(() => stuffExtendedResourceWritable(() => 'a')), + withResource(() => ({ + idWritable: stuffExtendedResourceWritable(() => 'a'), + idReadable: stuffExtendedResourceReadable(() => 'a'), + })), + ); + const unnamedWritableAndNamedComboStore = TestBed.inject( + UnnamedWritableAndNamedComboStore, + ); + + await wait(); + + expect(unnamedWritableAndNamedComboStore.value()).toBe('a'); + expect(unnamedWritableAndNamedComboStore.stuff()).toBe('a stuff'); + expect(unnamedWritableAndNamedComboStore.idWritableValue()).toBe('a'); + expect(unnamedWritableAndNamedComboStore.idReadableValue()).toBe('a'); + expect(unnamedWritableAndNamedComboStore.idWritableStuff()).toBe( + 'a stuff', + ); + expect(unnamedWritableAndNamedComboStore.idReadableStuff()).toBe( + 'a stuff', + ); + + patchState(unnamedWritableAndNamedComboStore, { + idWritableValue: 'b', + }); + patchState(unnamedWritableAndNamedComboStore, { value: 'b' }); + patchState(unnamedWritableAndNamedComboStore, { + // @ts-expect-error - readable resources should not have their value patchable + idReadableValue: 'b', + }); + + expect(unnamedWritableAndNamedComboStore.value()).toBe('b'); + expect(unnamedWritableAndNamedComboStore.stuff()).toBe('b stuff'); + expect(unnamedWritableAndNamedComboStore.idWritableValue()).toBe('b'); + expect(unnamedWritableAndNamedComboStore.idReadableValue()).toBe('a'); + expect(unnamedWritableAndNamedComboStore.idWritableStuff()).toBe( + 'b stuff', + ); + expect(unnamedWritableAndNamedComboStore.idReadableStuff()).toBe( + 'a stuff', + ); + + const UnnamedUnwritableAndNamedComboStore = signalStore( + { providedIn: 'root', protectedState: false }, + withResource(() => stuffExtendedResourceReadable(() => 'a')), + withResource(() => ({ + idWritable: stuffExtendedResourceWritable(() => 'a'), + idReadable: stuffExtendedResourceReadable(() => 'a'), + })), + ); + const unnamedUnwritableAndNamedComboStore = TestBed.inject( + UnnamedUnwritableAndNamedComboStore, + ); + + await wait(); + + expect(unnamedUnwritableAndNamedComboStore.value()).toBe('a'); + expect(unnamedUnwritableAndNamedComboStore.stuff()).toBe('a stuff'); + expect(unnamedUnwritableAndNamedComboStore.idWritableValue()).toBe( + 'a', + ); + expect(unnamedUnwritableAndNamedComboStore.idReadableValue()).toBe( + 'a', + ); + expect(unnamedUnwritableAndNamedComboStore.idWritableStuff()).toBe( + 'a stuff', + ); + expect(unnamedUnwritableAndNamedComboStore.idReadableStuff()).toBe( + 'a stuff', + ); + + patchState(unnamedUnwritableAndNamedComboStore, { + idWritableValue: 'b', + }); + // @ts-expect-error - readable resources should not have their value patchable + patchState(unnamedUnwritableAndNamedComboStore, { value: 'b' }); + patchState(unnamedUnwritableAndNamedComboStore, { + // @ts-expect-error - readable resources should not have their value patchable + idReadableValue: 'b', + }); + + expect(unnamedUnwritableAndNamedComboStore.value()).toBe('a'); + expect(unnamedUnwritableAndNamedComboStore.stuff()).toBe('a stuff'); + expect(unnamedUnwritableAndNamedComboStore.idWritableValue()).toBe( + 'b', + ); + expect(unnamedUnwritableAndNamedComboStore.idReadableValue()).toBe( + 'a', + ); + expect(unnamedUnwritableAndNamedComboStore.idWritableStuff()).toBe( + 'b stuff', + ); + expect(unnamedUnwritableAndNamedComboStore.idReadableStuff()).toBe( + 'a stuff', + ); + }); + }); + describe('override protection', () => { const warningSpy = jest.spyOn(console, 'warn'); @@ -312,6 +548,33 @@ describe('withResource', () => { 'userValue', ); }); + + //TODO wait for https://github.com/ngrx/platform/pull/4932 and then add 'value' to the list + it.each([ + 'status', + 'error', + 'isLoading', + '_reload', + 'hasValue', + 'stuff', + ])( + `warns if %s is not a member of the store with a custom extended resource`, + (memberName) => { + const Store = signalStore( + { providedIn: 'root' }, + withProps(() => ({ [memberName]: true })), + withResource(() => stuffExtendedResourceWritable(() => 'a')), + ); + + TestBed.inject(Store); + + expect(warningSpy).toHaveBeenCalledWith( + '@ngrx/signals: SignalStore members cannot be overridden.', + 'Trying to override:', + memberName, + ); + }, + ); }); it('works also with list/detail use case', async () => { @@ -319,12 +582,14 @@ describe('withResource', () => { { providedIn: 'root', protectedState: false }, withState({ id: undefined as number | undefined }), withResource(({ id }) => ({ - list: httpResource<{ id: number; name: string }[]>( - () => '/address', + list: httpResource< { - defaultValue: [], - }, - ), + id: number; + name: string; + }[] + >(() => '/address', { + defaultValue: [], + }), detail: httpResource
(() => id() ? `/address/${id()}` : undefined, ), @@ -609,6 +874,77 @@ describe('withResource', () => { IsEqual> >; }); + it('only exposes reload methods for reloadable resources', () => { + const Store = signalStore( + { providedIn: 'root' }, + withResource(() => ({ + digit: resource({ + loader: () => Promise.resolve(-1), + defaultValue: 0, + }), + digitSnapshotted: withPreviousValue( + resource({ + loader: () => Promise.resolve(-1), + defaultValue: 0, + }), + ), + })), + withResource(() => + resource({ + loader: () => Promise.resolve(-1), + defaultValue: 0, + }), + ), + withMethods((store) => ({ + namedReload: () => store._digitReload(), + unnamedReload: () => store._reload(), + // @ts-expect-error - a snapshot should not have a reload + invalidReload: () => store._digitSnapshottedReload(), + })), + ); + + const _store = TestBed.inject(Store); + + type _T1 = Assert boolean>>; + type _T2 = Assert boolean>>; + }); + + describe('patchState restrictions do not allow patching', () => { + it('an unnamed non-reloadable resource value', () => { + signalStore( + withResource(() => + withPreviousValue( + resource({ + loader: () => Promise.resolve(1), + defaultValue: 0, + }), + ), + ), + withMethods((store) => ({ + // @ts-expect-error - non-reloadable Resource values are not state + setValue: (value: number) => patchState(store, { value }), + })), + ); + }); + + it('a named non-reloadable resource value', () => { + signalStore( + withResource(() => ({ + digitSnapshotted: withPreviousValue( + resource({ + loader: () => Promise.resolve(-1), + defaultValue: 0, + }), + ), + })), + withMethods((store) => ({ + setNamedValue: (value: number) => + // @ts-expect-error digitSnapshotted is a `Resource` value, but not state, so it should not be patchable + patchState(store, { digitSnapshottedValue: value }), + })), + ); + }); + }); describe('mapToResource', () => { it('satisfies the Resource interface without default value', () => { diff --git a/libs/ngrx-toolkit/src/lib/with-resource.ts b/libs/ngrx-toolkit/src/lib/with-resource.ts index 5251935f..c7456308 100644 --- a/libs/ngrx-toolkit/src/lib/with-resource.ts +++ b/libs/ngrx-toolkit/src/lib/with-resource.ts @@ -1,9 +1,11 @@ //** Types for `withResource` */ +import { HttpResourceRef } from '@angular/common/http'; import { isSignal, Resource, ResourceRef, + ResourceSnapshot, ResourceStatus, Signal, untracked, @@ -25,6 +27,19 @@ export type ResourceResult = { status: Signal; error: Signal; isLoading: Signal; + snapshot: Signal>; + }; + methods: { + hasValue(): this is Resource>; + }; +}; +export type ResourceRefResult = { + state: { value: T }; + props: { + status: Signal; + error: Signal; + isLoading: Signal; + snapshot: Signal>; }; methods: { hasValue(): this is Resource>; @@ -32,33 +47,136 @@ export type ResourceResult = { }; }; -export type ResourceDictionary = Record>; +type ReloadableResource = ResourceRef | HttpResourceRef; + +type ResourceCoreKeys = + | 'value' + | 'status' + | 'error' + | 'isLoading' + | 'snapshot' + | 'hasValue'; + +type AdditionalSignalProps> = { + [Prop in keyof T as Prop extends ResourceCoreKeys + ? never + : T[Prop] extends Signal + ? Prop + : never]: T[Prop]; +}; + +type InferResourceValue> = + T['value'] extends Signal ? S : never; + +type ConditionalReloadMethod> = + T extends ReloadableResource + ? { _reload(): boolean } + : Record; + +declare const NON_PATCHABLE_RESOURCE_STATE: unique symbol; + +type NonPatchableResourceStateMarker = { + [NON_PATCHABLE_RESOURCE_STATE]?: never; +}; + +type ResourceValueType< + T extends Resource, + HasUndefinedErrorHandling extends boolean, +> = HasUndefinedErrorHandling extends true + ? InferResourceValue | undefined + : InferResourceValue; + +type UnnamedResourceResult< + T extends Resource, + HasUndefinedErrorHandling extends boolean, +> = { + state: T extends ReloadableResource + ? { + value: ResourceValueType; + } + : NonPatchableResourceStateMarker; + props: (T extends ReloadableResource + ? Record + : { + value: Signal>; + }) & { + status: Signal; + error: Signal; + isLoading: Signal; + snapshot: Signal>>; + } & AdditionalSignalProps; + methods: { + hasValue(): this is Resource, undefined>>; + } & ConditionalReloadMethod; +}; + +export type ResourceDictionary = Record>; + +// Without this, only one resource would be able to have its additional signal props extracted +// When there are two or more named resources, +// neither additional property can be indexed for certain, +// because it would be a union of the two's additional properties, +// and TS cannot pick the correct one when indexing into it. +// So there would be no resolution of a particular resource's additional property. +// This type util makes this type mapping an intersection, so all additional properties +// of all resources are preserved, but also can be indexed for a particular resource. +// https://stackoverflow.com/a/50375286 explanation, with relevant TS doc links +type UnionToIntersection = ( + T extends unknown ? (arg: T) => void : never +) extends (arg: infer R) => void + ? R + : Record; + +type NamedAdditionalSignalProps = + UnionToIntersection< + { + [ResourceName in keyof T & string]: { + [Prop in keyof AdditionalSignalProps & + string as `${ResourceName}${Capitalize}`]: AdditionalSignalProps< + T[ResourceName] + >[Prop]; + }; + }[keyof T & string] + >; export type NamedResourceResult< T extends ResourceDictionary, HasUndefinedErrorHandling extends boolean, > = { state: { - [Prop in keyof T as `${Prop & - string}Value`]: T[Prop]['value'] extends Signal - ? HasUndefinedErrorHandling extends true - ? S | undefined - : S + [Prop in keyof T as T[Prop] extends ReloadableResource + ? `${Prop & string}Value` + : never]: T[Prop] extends Resource + ? ResourceValueType : never; - }; + } & NonPatchableResourceStateMarker; props: { + [Prop in keyof T as T[Prop] extends ReloadableResource + ? never + : `${Prop & string}Value`]: Signal< + T[Prop] extends Resource + ? ResourceValueType + : never + >; + } & { [Prop in keyof T as `${Prop & string}Status`]: Signal; } & { [Prop in keyof T as `${Prop & string}Error`]: Signal; } & { [Prop in keyof T as `${Prop & string}IsLoading`]: Signal; - }; + } & { + [Prop in keyof T as `${Prop & string}Snapshot`]: Signal< + ResourceSnapshot ? S : never> + >; + } & NamedAdditionalSignalProps; methods: { [Prop in keyof T as `${Prop & string}HasValue`]: () => this is Resource< - Exclude + Exclude, undefined> >; } & { - [Prop in keyof T as `_${Prop & string}Reload`]: () => boolean; + [Prop in keyof T as T[Prop] extends ReloadableResource + ? `_${Prop & string}Reload` + : never]: () => boolean; }; }; @@ -81,10 +199,13 @@ const defaultOptions: Required = { * Integrates a `Resource` into the SignalStore and makes the store instance * implement the `Resource` interface. * - * The resource's value is stored under the `value` key in the state - * and is exposed as a `DeepSignal`. + * Reloadable resources (`ResourceRef`/`HttpResourceRef`) expose their + * value under `value` in the store state and can be updated via `patchState`. * - * It can also be updated via `patchState`. + * Plain `Resource` values are exposed as signals on the store instance, + * but are not part of state and therefore cannot be changed via `patchState`. + * Additional signal members on the resource (for example `foo`) are also + * exposed on the store as props. * * @usageNotes * @@ -108,32 +229,32 @@ const defaultOptions: Required = { */ export function withResource< Input extends SignalStoreFeatureResult, - ResourceValue, + ResourceType extends Resource, >( resourceFactory: ( store: Input['props'] & Input['methods'] & StateSignals, - ) => ResourceRef, -): SignalStoreFeature>; + ) => ResourceType, +): SignalStoreFeature>; export function withResource< Input extends SignalStoreFeatureResult, - ResourceValue, + ResourceType extends Resource, >( resourceFactory: ( store: Input['props'] & Input['methods'] & StateSignals, - ) => ResourceRef, + ) => ResourceType, resourceOptions: { errorHandling: 'undefined value' }, -): SignalStoreFeature>; +): SignalStoreFeature>; export function withResource< Input extends SignalStoreFeatureResult, - ResourceValue, + ResourceType extends Resource, >( resourceFactory: ( store: Input['props'] & Input['methods'] & StateSignals, - ) => ResourceRef, + ) => ResourceType, resourceOptions?: ResourceOptions, -): SignalStoreFeature>; +): SignalStoreFeature>; /** * @experimental @@ -143,9 +264,13 @@ export function withResource< * registered by name, which is used as a prefix when spreading the members * of `Resource` onto the store. * - * Each resource’s value is part of the state, stored under the `value` key - * with the resource name as prefix. Values are exposed as `DeepSignal`s and - * can be updated via `patchState`. + * Reloadable resources (`ResourceRef`/`HttpResourceRef`) place their values + * in state using `Value` keys and support updates via `patchState`. + * + * Plain `Resource` values are exposed as read-only signals with `Value` + * but are not stored in state. + * Additional signal members are exposed as read-only props using + * ``. * * @usageNotes * @@ -166,7 +291,7 @@ export function withResource< * ``` * * @param resourceFactory A factory function that receives the store's props, - * methods, and state signals. It must return a `Record`. + * methods, and state signals. It must return a `Record`. * @param resourceOptions Allows to configure the error handling behavior. */ export function withResource< @@ -204,7 +329,7 @@ export function withResource< >( resourceFactory: ( store: Input['props'] & Input['methods'] & StateSignals, - ) => ResourceRef | ResourceDictionary, + ) => Resource | ResourceDictionary, resourceOptions?: ResourceOptions, ): SignalStoreFeature { const options: Required = { @@ -218,7 +343,10 @@ export function withResource< ...store.methods, }); - if (isResourceRef(resourceOrDictionary)) { + if ( + isResourceRef(resourceOrDictionary) || + isResource(resourceOrDictionary) + ) { return createUnnamedResource( resourceOrDictionary, options.errorHandling, @@ -233,26 +361,44 @@ export function withResource< } function createUnnamedResource( - resource: ResourceRef, + resource: Resource, errorHandling: ErrorHandling, ) { function hasValue(): this is Resource> { - return resource.hasValue(); + if (isResourceRef(resource)) { + return resource.hasValue(); + } else { + return resource.hasValue(); + } + } + + const metadataProps = { + status: resource.status, + error: resource.error, + isLoading: resource.isLoading, + snapshot: resource.snapshot, + ...extractAdditionalSignalProps(resource), + }; + + if (isResourceRef(resource)) { + return signalStoreFeature( + withLinkedState(() => ({ + value: valueSignalForErrorHandling(resource, errorHandling), + })), + withProps(() => metadataProps), + withMethods(() => ({ + hasValue, + _reload: () => resource.reload(), + })), + ); } return signalStoreFeature( - withLinkedState(() => ({ - value: valueSignalForErrorHandling(resource, errorHandling), - })), withProps(() => ({ - status: resource.status, - error: resource.error, - isLoading: resource.isLoading, - })), - withMethods(() => ({ - hasValue, - _reload: () => resource.reload(), + value: valueSignalForErrorHandling(resource, errorHandling), + ...metadataProps, })), + withMethods(() => ({ hasValue })), ); } @@ -262,37 +408,42 @@ function createNamedResource( ) { const keys = Object.keys(dictionary); - const state: Record> = keys.reduce( - (state, resourceName) => ({ - ...state, - [`${resourceName}Value`]: valueSignalForErrorHandling( - dictionary[resourceName], + const state: Record> = {}; + const props: Record> = {}; + const methods: Record boolean> = {}; + + for (const resourceName of keys) { + const res = dictionary[resourceName]; + + props[`${resourceName}Status`] = res.status; + props[`${resourceName}Error`] = res.error; + props[`${resourceName}IsLoading`] = res.isLoading; + props[`${resourceName}Snapshot`] = res.snapshot; + assignPrefixedProps(props, resourceName, extractAdditionalSignalProps(res)); + methods[`${resourceName}HasValue`] = isResourceRef(res) + ? () => res.hasValue() + : () => res.hasValue(); + + if (isResourceRef(res)) { + state[`${resourceName}Value`] = valueSignalForErrorHandling( + res, errorHandling, - ), - }), - {}, - ); - - const props: Record> = keys.reduce( - (props, resourceName) => ({ - ...props, - [`${resourceName}Status`]: dictionary[resourceName].status, - [`${resourceName}Error`]: dictionary[resourceName].error, - [`${resourceName}IsLoading`]: dictionary[resourceName].isLoading, - }), - {}, - ); + ) as WritableSignal; + methods[`_${resourceName}Reload`] = () => res.reload(); + } else { + props[`${resourceName}Value`] = valueSignalForErrorHandling( + res, + errorHandling, + ); + } + } - const methods: Record boolean> = keys.reduce( - (methods, resourceName) => { - return { - ...methods, - [`${resourceName}HasValue`]: () => dictionary[resourceName].hasValue(), - [`_${resourceName}Reload`]: () => dictionary[resourceName].reload(), - }; - }, - {}, - ); + if (Object.keys(state).length === 0) { + return signalStoreFeature( + withProps(() => props), + withMethods(() => methods), + ); + } return signalStoreFeature( withLinkedState(() => state), @@ -301,7 +452,7 @@ function createNamedResource( ); } -export function isResourceRef(value: unknown): value is ResourceRef { +export function isResource(value: unknown): value is Resource { return ( value !== null && typeof value === 'object' && @@ -310,11 +461,81 @@ export function isResourceRef(value: unknown): value is ResourceRef { 'status' in value && 'error' in value && 'isLoading' in value && - 'hasValue' in value && - 'reload' in value + 'snapshot' in value && + 'hasValue' in value + ); +} + +export function isResourceRef(value: unknown): value is ResourceRef { + return ( + isResource(value) && + 'reload' in value && + 'set' in value && + 'update' in value && + 'asReadonly' in value ); } +function extractAdditionalSignalProps( + resource: Resource, +): Record> { + const additionalSignals: Record> = {}; + + for (const key of Object.keys(resource)) { + if (isResourceCoreKey(key)) { + continue; + } + + const candidate = (resource as unknown as Record)[key]; + if (isSignal(candidate)) { + additionalSignals[key] = candidate; + } + } + + return additionalSignals; +} + +function isResourceCoreKey(key: string): key is ResourceCoreKeys { + return ( + key === 'value' || + key === 'status' || + key === 'error' || + key === 'isLoading' || + key === 'snapshot' || + key === 'hasValue' + ); +} + +function assignPrefixedProps( + target: Record>, + resourceName: string, + source: Record>, +): void { + for (const key of Object.keys(source)) { + target[`${resourceName}${capitalizeKey(key)}`] = source[key]; + } +} + +function capitalizeKey(value: string): string { + if (value.length === 0) { + return value; + } + + return value.charAt(0).toUpperCase() + value.slice(1); +} + +// This may be handy in the future +// export function isHttpResourceRef( +// value: unknown, +// ): value is HttpResourceRef { +// return ( +// isResourceRef(value) && +// 'headers' in value && +// 'statusCode' in value && +// 'progress' in value +// ); +// } + //** Types for `mapToResource` */ type NamedResource = { @@ -327,6 +548,8 @@ type NamedResource = { [Prop in `${Name}IsLoading`]: Signal; } & { [Prop in `${Name}HasValue`]: () => boolean; +} & { + [Prop in `${Name}Snapshot`]: Signal>; }; type IsValidResourceName< @@ -375,7 +598,7 @@ type MappedResource< * * @param store The store instance to map the resource to. * @param name The name of the resource to map. - * @returns `ResourceRef` + * @returns `Resource` */ export function mapToResource< Name extends ResourceNames, @@ -394,6 +617,7 @@ export function mapToResource< status: store[`${resourceName}Status`], error: store[`${resourceName}Error`], isLoading: store[`${resourceName}IsLoading`], + snapshot: store[`${resourceName}Snapshot`], hasValue, } as MappedResource; } @@ -464,19 +688,19 @@ export function mapToResource< * a breaking change. */ function valueSignalForErrorHandling( - res: ResourceRef, + res: Resource, errorHandling: 'undefined value', -): WritableSignal; +): Signal; function valueSignalForErrorHandling( - res: ResourceRef, + res: Resource, errorHandling: ErrorHandling, -): WritableSignal; +): Signal; function valueSignalForErrorHandling( - res: ResourceRef, + res: Resource, errorHandling: ErrorHandling, -): WritableSignal { +): Signal { const originalSignal = res.value; switch (errorHandling) { diff --git a/libs/ngrx-toolkit/src/lib/with-resource/tests/util/custom-extended-resources.ts b/libs/ngrx-toolkit/src/lib/with-resource/tests/util/custom-extended-resources.ts new file mode 100644 index 00000000..469991b3 --- /dev/null +++ b/libs/ngrx-toolkit/src/lib/with-resource/tests/util/custom-extended-resources.ts @@ -0,0 +1,54 @@ +import { computed, Resource, ResourceRef, Signal } from '@angular/core'; +import { rxResource } from '@angular/core/rxjs-interop'; +import { of } from 'rxjs'; + +export function stuffExtendedResourceWritable( + params?: () => string | undefined, +): ResourceRef & { stuff: Signal } { + const resource = rxResource({ + params: () => params?.() ?? '', + stream: ({ params }) => { + return of(params); + }, + }); + + const stuff = computed(() => resource.value() + ' stuff'); + + return { + stuff, + value: resource.value, + status: resource.status, + error: resource.error, + isLoading: resource.isLoading, + snapshot: resource.snapshot, + hasValue: resource.hasValue, + set: resource.set.bind(resource), + update: resource.update.bind(resource), + asReadonly: resource.asReadonly.bind(resource), + reload: resource.reload.bind(resource), + destroy: resource.destroy.bind(resource), + }; +} + +export function stuffExtendedResourceReadable( + params?: () => string | undefined, +): Resource & { stuff: Signal } { + const resource = rxResource({ + params: () => params?.() ?? '', + stream: ({ params }) => { + return of(params); + }, + }).asReadonly(); + + const stuff = computed(() => resource.value() + ' stuff'); + + return { + stuff, + value: resource.value, + status: resource.status, + error: resource.error, + isLoading: resource.isLoading, + snapshot: resource.snapshot, + hasValue: resource.hasValue.bind(resource), + }; +} diff --git a/libs/ngrx-toolkit/src/lib/with-resource/tests/util/resource-test-adapter.ts b/libs/ngrx-toolkit/src/lib/with-resource/tests/util/resource-test-adapter.ts index d0cc9457..f874c9bb 100644 --- a/libs/ngrx-toolkit/src/lib/with-resource/tests/util/resource-test-adapter.ts +++ b/libs/ngrx-toolkit/src/lib/with-resource/tests/util/resource-test-adapter.ts @@ -12,5 +12,8 @@ export type ResourceTestAdapter = { isLoading: boolean; hasValue: boolean; }; +}; + +export type ReloadableResourceTestAdapter = ResourceTestAdapter & { reload: () => void; }; diff --git a/libs/ngrx-toolkit/src/lib/with-resource/tests/util/setup-mapped-resource.ts b/libs/ngrx-toolkit/src/lib/with-resource/tests/util/setup-mapped-resource.ts index 6f0af070..bc015ba5 100644 --- a/libs/ngrx-toolkit/src/lib/with-resource/tests/util/setup-mapped-resource.ts +++ b/libs/ngrx-toolkit/src/lib/with-resource/tests/util/setup-mapped-resource.ts @@ -9,7 +9,7 @@ import { setupNamedResource } from './setup-named-resource'; export function setupMappedResource( errorHandling: ErrorHandling, ): ResourceTestAdapter { - const { addressResolver, setId, setValue, reload, store } = + const { addressResolver, setId, setValue, store } = setupNamedResource(errorHandling); const resource = mapToResource( @@ -28,6 +28,5 @@ export function setupMappedResource( isLoading: resource.isLoading(), hasValue: resource.hasValue(), }), - reload, }; } diff --git a/libs/ngrx-toolkit/src/lib/with-resource/tests/util/setup-named-resource.ts b/libs/ngrx-toolkit/src/lib/with-resource/tests/util/setup-named-resource.ts index 2dc4a969..ca5f1858 100644 --- a/libs/ngrx-toolkit/src/lib/with-resource/tests/util/setup-named-resource.ts +++ b/libs/ngrx-toolkit/src/lib/with-resource/tests/util/setup-named-resource.ts @@ -1,6 +1,6 @@ import { inject, resource } from '@angular/core'; import { TestBed } from '@angular/core/testing'; -import { patchState, signalStore, withMethods, withState } from '@ngrx/signals'; +import { patchState, signalStore, withState } from '@ngrx/signals'; import { ErrorHandling, withResource } from '../../../with-resource'; import { Address, AddressResolver, venice } from './fixtures'; import { ResourceTestAdapter } from './resource-test-adapter'; @@ -27,7 +27,6 @@ export function setupNamedResource( }, { errorHandling }, ), - withMethods((store) => ({ reload: () => store._addressReload() })), ); TestBed.configureTestingModule({ @@ -54,6 +53,5 @@ export function setupNamedResource( isLoading: store.addressIsLoading(), hasValue: store.addressHasValue(), }), - reload: () => store.reload(), }; } diff --git a/libs/ngrx-toolkit/src/lib/with-resource/tests/util/setup-unnamed-resource.ts b/libs/ngrx-toolkit/src/lib/with-resource/tests/util/setup-unnamed-resource.ts index 0939d10d..c3b075cb 100644 --- a/libs/ngrx-toolkit/src/lib/with-resource/tests/util/setup-unnamed-resource.ts +++ b/libs/ngrx-toolkit/src/lib/with-resource/tests/util/setup-unnamed-resource.ts @@ -3,11 +3,11 @@ import { TestBed } from '@angular/core/testing'; import { patchState, signalStore, withMethods, withState } from '@ngrx/signals'; import { ErrorHandling, withResource } from '../../../with-resource'; import { Address, AddressResolver, venice } from './fixtures'; -import { ResourceTestAdapter } from './resource-test-adapter'; +import { ReloadableResourceTestAdapter } from './resource-test-adapter'; export function setupUnnamedResource( errorHandling?: ErrorHandling, -): ResourceTestAdapter { +): ReloadableResourceTestAdapter { const addressResolver = { resolve: jest.fn(() => Promise.resolve(venice)), }; diff --git a/libs/ngrx-toolkit/src/lib/with-resource/tests/util/snapshot.ts b/libs/ngrx-toolkit/src/lib/with-resource/tests/util/snapshot.ts new file mode 100644 index 00000000..57829184 --- /dev/null +++ b/libs/ngrx-toolkit/src/lib/with-resource/tests/util/snapshot.ts @@ -0,0 +1,26 @@ +import { + linkedSignal, + Resource, + resourceFromSnapshots, + ResourceSnapshot, +} from '@angular/core'; + +export function withPreviousValue(input: Resource): Resource { + const derived = linkedSignal, ResourceSnapshot>({ + source: input.snapshot, + computation: (snap, previous) => { + if ( + snap.status === 'loading' && + previous && + previous.value.status !== 'error' + ) { + // When the input resource enters loading state, we keep the value + // from its previous state, if any. + return { status: 'loading' as const, value: previous.value.value }; + } + // Otherwise we simply forward the state of the input resource. + return snap; + }, + }); + return resourceFromSnapshots(derived); +}