From a594d3215ca956bb60d415a30269acf23c8858b3 Mon Sep 17 00:00:00 2001 From: Rainer Hahnekamp Date: Sun, 16 Nov 2025 10:20:38 +0100 Subject: [PATCH 1/3] feat: add parts --- .../src/lib/with-resource.spec.ts | 442 +++++++++--------- libs/ngrx-toolkit/src/lib/with-resource.ts | 85 +++- 2 files changed, 314 insertions(+), 213 deletions(-) diff --git a/libs/ngrx-toolkit/src/lib/with-resource.spec.ts b/libs/ngrx-toolkit/src/lib/with-resource.spec.ts index 82e6b2d5..3e28049c 100644 --- a/libs/ngrx-toolkit/src/lib/with-resource.spec.ts +++ b/libs/ngrx-toolkit/src/lib/with-resource.spec.ts @@ -14,7 +14,7 @@ import { withState, } from '@ngrx/signals'; import { of } from 'rxjs'; -import { mapToResource, withResource } from './with-resource'; +import { ErrorHandling, mapToResource, withResource } from './with-resource'; describe('withResource', () => { describe('standard tests', () => { @@ -46,20 +46,23 @@ describe('withResource', () => { } } - function setupWithUnnamedResource() { + function setupWithUnnamedResource(errorHandling: ErrorHandling) { const addressResolver = { resolve: jest.fn(() => Promise.resolve(venice)), }; const AddressStore = signalStore( { providedIn: 'root', protectedState: false }, withState({ id: undefined as number | undefined }), - withResource((store) => { - const resolver = inject(AddressResolver); - return resource({ - params: store.id, - loader: ({ params: id }) => resolver.resolve(id), - }); - }), + withResource( + (store) => { + const resolver = inject(AddressResolver); + return resource({ + params: store.id, + loader: ({ params: id }) => resolver.resolve(id), + }); + }, + { errorHandling }, + ), withMethods((store) => ({ reload: () => store._reload() })), ); @@ -77,7 +80,7 @@ describe('withResource', () => { return { store, addressResolver }; } - function setupWithNamedResource() { + function setupWithNamedResource(errorHandling: ErrorHandling) { const addressResolver = { resolve: jest.fn(() => Promise.resolve(venice)), }; @@ -85,15 +88,18 @@ describe('withResource', () => { const UserStore = signalStore( { providedIn: 'root', protectedState: false }, withState({ id: undefined as number | undefined }), - withResource((store) => { - const resolver = inject(AddressResolver); - return { - address: resource({ - params: store.id, - loader: ({ params: id }) => resolver.resolve(id), - }), - }; - }), + withResource( + (store) => { + const resolver = inject(AddressResolver); + return { + address: resource({ + params: store.id, + loader: ({ params: id }) => resolver.resolve(id), + }), + }; + }, + { errorHandling }, + ), withMethods((store) => ({ reload: () => store._addressReload() })), ); @@ -185,208 +191,226 @@ describe('withResource', () => { }); describe('status checks', () => { - for (const { name, setup } of [ - { - name: 'unnamed resource', - setup: () => { - const { store, addressResolver } = setupWithUnnamedResource(); - const setId = (id: number) => patchState(store, { id }); - const setValue = (value: Address) => patchState(store, { value }); - return { - storeAndResource: store, - addressResolver, - setId, - setValue, - }; - }, - }, - { - name: 'mapped named resource', - setup: () => { - const { store, addressResolver } = setupWithNamedResource(); - const storeAndResource = mapToResource(store, 'address'); - const setId = (id: number) => patchState(store, { id }); - const setValue = (value: Address) => - patchState(store, { addressValue: value }); - return { storeAndResource, addressResolver, setId, setValue }; - }, - }, - ]) { - describe(name, () => { - it('has idle status in the beginning', () => { - const { storeAndResource } = setup(); - - expect(storeAndResource.status()).toBe('idle'); - expect(storeAndResource.value()).toBeUndefined(); - expect(storeAndResource.error()).toBeUndefined(); - expect(storeAndResource.isLoading()).toBe(false); - expect(storeAndResource.hasValue()).toBe(false); - }); - - it('has loading status when loading', () => { - const { storeAndResource, addressResolver, setId } = setup(); - - addressResolver.resolve.mockResolvedValue(venice); - setId(1); - - expect(storeAndResource.status()).toBe('loading'); - expect(storeAndResource.value()).toBeUndefined(); - expect(storeAndResource.error()).toBeUndefined(); - expect(storeAndResource.isLoading()).toBe(true); - expect(storeAndResource.hasValue()).toBe(false); - }); - - it('has resolved status when loaded', async () => { - const { storeAndResource, addressResolver, setId } = setup(); + const errorHandlings: ErrorHandling[] = [ + 'native', + 'undefined value', + 'previous value', + ]; + for (const errorHandling of errorHandlings) { + describe(`Error Handling: ${errorHandling}`, () => { + for (const { name, setup } of [ + { + name: 'unnamed resource', + setup: () => { + const { store, addressResolver } = + setupWithUnnamedResource(errorHandling); + const setId = (id: number) => patchState(store, { id }); + const setValue = (value: Address) => + patchState(store, { value }); + return { + storeAndResource: store, + addressResolver, + setId, + setValue, + }; + }, + }, + { + name: 'mapped named resource', + setup: () => { + const { store, addressResolver } = + setupWithNamedResource(errorHandling); + const storeAndResource = mapToResource(store, 'address'); + const setId = (id: number) => patchState(store, { id }); + const setValue = (value: Address) => + patchState(store, { addressValue: value }); + return { storeAndResource, addressResolver, setId, setValue }; + }, + }, + ]) { + describe(name, () => { + it('has idle status in the beginning', () => { + const { storeAndResource } = setup(); + + expect(storeAndResource.status()).toBe('idle'); + expect(storeAndResource.value()).toBeUndefined(); + expect(storeAndResource.error()).toBeUndefined(); + expect(storeAndResource.isLoading()).toBe(false); + expect(storeAndResource.hasValue()).toBe(false); + }); + + it('has loading status when loading', () => { + const { storeAndResource, addressResolver, setId } = setup(); + + addressResolver.resolve.mockResolvedValue(venice); + setId(1); + + expect(storeAndResource.status()).toBe('loading'); + expect(storeAndResource.value()).toBeUndefined(); + expect(storeAndResource.error()).toBeUndefined(); + expect(storeAndResource.isLoading()).toBe(true); + expect(storeAndResource.hasValue()).toBe(false); + }); + + it('has resolved status when loaded', async () => { + const { storeAndResource, addressResolver, setId } = setup(); + + addressResolver.resolve.mockResolvedValue(venice); + setId(1); + + await wait(); + + expect(storeAndResource.status()).toBe('resolved'); + expect(storeAndResource.value()).toEqual(venice); + expect(storeAndResource.error()).toBeUndefined(); + expect(storeAndResource.isLoading()).toBe(false); + expect(storeAndResource.hasValue()).toBe(true); + }); + + it('has error status when error', async () => { + const { storeAndResource, addressResolver, setId } = setup(); + + addressResolver.resolve.mockRejectedValue(new Error('Error')); + setId(1); + await wait(); + + expect(storeAndResource.status()).toBe('error'); + expect(() => storeAndResource.value()).toThrow(); + expect(storeAndResource.error()).toBeInstanceOf(Error); + expect(storeAndResource.isLoading()).toBe(false); + expect(storeAndResource.hasValue()).toBe(false); + }); + + it('has local once updated', async () => { + const { storeAndResource, addressResolver, setId, setValue } = + setup(); + + addressResolver.resolve.mockResolvedValue(venice); + setId(1); + + await wait(); + setValue({ ...venice, country: 'Italia' }); + + expect(storeAndResource.status()).toBe('local'); + expect(storeAndResource.value()?.country).toBe('Italia'); + expect(storeAndResource.error()).toBeUndefined(); + expect(storeAndResource.isLoading()).toBe(false); + expect(storeAndResource.hasValue()).toBe(true); + }); + }); + } + + it('reloads an unnamed resource', async () => { + const { store, addressResolver } = + setupWithUnnamedResource(errorHandling); addressResolver.resolve.mockResolvedValue(venice); - setId(1); + patchState(store, { id: 1 }); await wait(); + expect(store.hasValue()).toBe(true); - expect(storeAndResource.status()).toBe('resolved'); - expect(storeAndResource.value()).toEqual(venice); - expect(storeAndResource.error()).toBeUndefined(); - expect(storeAndResource.isLoading()).toBe(false); - expect(storeAndResource.hasValue()).toBe(true); - }); - - it('has error status when error', async () => { - const { storeAndResource, addressResolver, setId } = setup(); + addressResolver.resolve.mockResolvedValue({ + ...venice, + country: 'Great Britain', + }); + store.reload(); - addressResolver.resolve.mockRejectedValue(new Error('Error')); - setId(1); await wait(); - - expect(storeAndResource.status()).toBe('error'); - expect(() => storeAndResource.value()).toThrow(); - expect(storeAndResource.error()).toBeInstanceOf(Error); - expect(storeAndResource.isLoading()).toBe(false); - expect(storeAndResource.hasValue()).toBe(false); + expect(store.value()?.country).toBe('Great Britain'); }); - it('has local once updated', async () => { - const { storeAndResource, addressResolver, setId, setValue } = - setup(); - - addressResolver.resolve.mockResolvedValue(venice); - setId(1); - - await wait(); - setValue({ ...venice, country: 'Italia' }); - - expect(storeAndResource.status()).toBe('local'); - expect(storeAndResource.value()?.country).toBe('Italia'); - expect(storeAndResource.error()).toBeUndefined(); - expect(storeAndResource.isLoading()).toBe(false); - expect(storeAndResource.hasValue()).toBe(true); + describe('named resource', () => { + it('has idle status in the beginning', () => { + const { store } = setupWithNamedResource(errorHandling); + + expect(store.addressStatus()).toBe('idle'); + expect(store.addressValue()).toBeUndefined(); + expect(store.addressError()).toBeUndefined(); + expect(store.addressIsLoading()).toBe(false); + expect(store.addressHasValue()).toBe(false); + }); + + it('has loading status when loading', () => { + const { store, addressResolver } = + setupWithNamedResource(errorHandling); + + addressResolver.resolve.mockResolvedValue(venice); + patchState(store, { id: 1 }); + + expect(store.addressStatus()).toBe('loading'); + expect(store.addressValue()).toBeUndefined(); + expect(store.addressError()).toBeUndefined(); + expect(store.addressIsLoading()).toBe(true); + expect(store.addressHasValue()).toBe(false); + }); + + it('has resolved status when loaded', async () => { + const { store, addressResolver } = + setupWithNamedResource(errorHandling); + + addressResolver.resolve.mockResolvedValue(venice); + patchState(store, { id: 1 }); + + await wait(); + + expect(store.addressStatus()).toBe('resolved'); + expect(store.addressValue()).toEqual(venice); + expect(store.addressError()).toBeUndefined(); + expect(store.addressIsLoading()).toBe(false); + expect(store.addressHasValue()).toBe(true); + }); + + it('has error status when error', async () => { + const { store, addressResolver } = + setupWithNamedResource(errorHandling); + + addressResolver.resolve.mockRejectedValue(new Error('Error')); + patchState(store, { id: 1 }); + await wait(); + + expect(store.addressStatus()).toBe('error'); + expect(() => store.addressValue()).toThrow(); + expect(store.addressError()).toBeInstanceOf(Error); + expect(store.addressIsLoading()).toBe(false); + expect(store.addressHasValue()).toBe(false); + }); + + it('has local once updated', async () => { + const { store, addressResolver } = + setupWithNamedResource(errorHandling); + + addressResolver.resolve.mockResolvedValue(venice); + patchState(store, { id: 1 }); + + await wait(); + patchState(store, ({ addressValue }) => ({ + addressValue: addressValue + ? { ...addressValue, country: 'Italia' } + : undefined, + })); + + expect(store.addressStatus()).toBe('local'); + expect(store.addressValue()?.country).toBe('Italia'); + expect(store.addressError()).toBeUndefined(); + expect(store.addressIsLoading()).toBe(false); + expect(store.addressHasValue()).toBe(true); + }); + + it('can also reload by resource name', async () => { + const { store, addressResolver } = + setupWithNamedResource(errorHandling); + + addressResolver.resolve.mockResolvedValueOnce(venice); + patchState(store, { id: 1 }); + await wait(); + expect(store.addressStatus()).toBe('resolved'); + store.reload(); + expect(store.addressStatus()).toBe('reloading'); + }); }); }); } - - it('reloads an unnamed resource', async () => { - const { store, addressResolver } = setupWithUnnamedResource(); - - addressResolver.resolve.mockResolvedValue(venice); - patchState(store, { id: 1 }); - - await wait(); - expect(store.hasValue()).toBe(true); - - addressResolver.resolve.mockResolvedValue({ - ...venice, - country: 'Great Britain', - }); - store.reload(); - - await wait(); - expect(store.value()?.country).toBe('Great Britain'); - }); - - describe('named resource', () => { - it('has idle status in the beginning', () => { - const { store } = setupWithNamedResource(); - - expect(store.addressStatus()).toBe('idle'); - expect(store.addressValue()).toBeUndefined(); - expect(store.addressError()).toBeUndefined(); - expect(store.addressIsLoading()).toBe(false); - expect(store.addressHasValue()).toBe(false); - }); - - it('has loading status when loading', () => { - const { store, addressResolver } = setupWithNamedResource(); - - addressResolver.resolve.mockResolvedValue(venice); - patchState(store, { id: 1 }); - - expect(store.addressStatus()).toBe('loading'); - expect(store.addressValue()).toBeUndefined(); - expect(store.addressError()).toBeUndefined(); - expect(store.addressIsLoading()).toBe(true); - expect(store.addressHasValue()).toBe(false); - }); - - it('has resolved status when loaded', async () => { - const { store, addressResolver } = setupWithNamedResource(); - - addressResolver.resolve.mockResolvedValue(venice); - patchState(store, { id: 1 }); - - await wait(); - - expect(store.addressStatus()).toBe('resolved'); - expect(store.addressValue()).toEqual(venice); - expect(store.addressError()).toBeUndefined(); - expect(store.addressIsLoading()).toBe(false); - expect(store.addressHasValue()).toBe(true); - }); - - it('has error status when error', async () => { - const { store, addressResolver } = setupWithNamedResource(); - - addressResolver.resolve.mockRejectedValue(new Error('Error')); - patchState(store, { id: 1 }); - await wait(); - - expect(store.addressStatus()).toBe('error'); - expect(() => store.addressValue()).toThrow(); - expect(store.addressError()).toBeInstanceOf(Error); - expect(store.addressIsLoading()).toBe(false); - expect(store.addressHasValue()).toBe(false); - }); - - it('has local once updated', async () => { - const { store, addressResolver } = setupWithNamedResource(); - - addressResolver.resolve.mockResolvedValue(venice); - patchState(store, { id: 1 }); - - await wait(); - patchState(store, ({ addressValue }) => ({ - addressValue: addressValue - ? { ...addressValue, country: 'Italia' } - : undefined, - })); - - expect(store.addressStatus()).toBe('local'); - expect(store.addressValue()?.country).toBe('Italia'); - expect(store.addressError()).toBeUndefined(); - expect(store.addressIsLoading()).toBe(false); - expect(store.addressHasValue()).toBe(true); - }); - - it('can also reload by resource name', async () => { - const { store, addressResolver } = setupWithNamedResource(); - - addressResolver.resolve.mockResolvedValueOnce(venice); - patchState(store, { id: 1 }); - await wait(); - expect(store.addressStatus()).toBe('resolved'); - store.reload(); - expect(store.addressStatus()).toBe('reloading'); - }); - }); }); describe('override protection', () => { diff --git a/libs/ngrx-toolkit/src/lib/with-resource.ts b/libs/ngrx-toolkit/src/lib/with-resource.ts index c2011b29..fd54fd6c 100644 --- a/libs/ngrx-toolkit/src/lib/with-resource.ts +++ b/libs/ngrx-toolkit/src/lib/with-resource.ts @@ -2,6 +2,7 @@ import { isSignal, + linkedSignal, Resource, ResourceRef, ResourceStatus, @@ -54,6 +55,16 @@ export type NamedResourceResult = { }; }; +export type ErrorHandling = 'native' | 'undefined value' | 'previous value'; + +export type ResourceOptions = { + errorHandling?: ErrorHandling; +}; + +const defaultOptions: Required = { + errorHandling: 'undefined value', +}; + //** Implementation of `withResource` */ /** @@ -94,6 +105,7 @@ export function withResource< resourceFactory: ( store: Input['props'] & Input['methods'] & StateSignals, ) => ResourceRef, + resourceOptions?: ResourceOptions, ): SignalStoreFeature>; /** @@ -136,6 +148,7 @@ export function withResource< resourceFactory: ( store: Input['props'] & Input['methods'] & StateSignals, ) => Dictionary, + resourceOptions?: ResourceOptions, ): SignalStoreFeature>; export function withResource< @@ -145,7 +158,12 @@ export function withResource< resourceFactory: ( store: Input['props'] & Input['methods'] & StateSignals, ) => ResourceRef | ResourceDictionary, + resourceOptions?: ResourceOptions, ): SignalStoreFeature { + const options: Required = { + ...defaultOptions, + ...(resourceOptions || {}), + }; return (store) => { const resourceOrDictionary = resourceFactory({ ...store.stateSignals, @@ -154,22 +172,34 @@ export function withResource< }); if (isResourceRef(resourceOrDictionary)) { - return createUnnamedResource(resourceOrDictionary)(store); + return createUnnamedResource( + resourceOrDictionary, + options.errorHandling, + )(store); } else { - return createNamedResource(resourceOrDictionary)(store); + return createNamedResource( + resourceOrDictionary, + options.errorHandling, + )(store); } }; } function createUnnamedResource( resource: ResourceRef, + errorHandling: ErrorHandling, ) { function hasValue(): this is Resource> { return resource.hasValue(); } return signalStoreFeature( - withLinkedState(() => ({ value: resource.value })), + withLinkedState(() => ({ + value: valueSignalForErrorHandling( + resource, + errorHandling, + ) as WritableSignal, + })), withProps(() => ({ status: resource.status, error: resource.error, @@ -184,13 +214,17 @@ function createUnnamedResource( function createNamedResource( dictionary: Dictionary, + errorHandling: ErrorHandling, ) { const keys = Object.keys(dictionary); const state: Record> = keys.reduce( (state, resourceName) => ({ ...state, - [`${resourceName}Value`]: dictionary[resourceName].value, + [`${resourceName}Value`]: valueSignalForErrorHandling( + dictionary[resourceName], + errorHandling, + ), }), {}, ); @@ -319,3 +353,46 @@ export function mapToResource< hasValue, } as MappedResource; } + +// We require an explicit error symbol, to clearly communicate +// that an error has happened. Since a resource's value can be +// any value, we're ensuring here that an error of true is not +// mistakenly seen as value. +const ERROR = Symbol('ERROR'); + +// Tests need to check if local changes are also executed. +// Test needs to check if we go directly from initial into value. what happens if default value is there, what happens if undefined. +function valueSignalForErrorHandling( + res: ResourceRef, + errorHandling: ErrorHandling, +): WritableSignal { + switch (errorHandling) { + case 'native': + return res.value; + case 'undefined value': + return linkedSignal(() => + res.status() === 'error' ? undefined : res.value(), + ); + case 'previous value': + return linkedSignal({ + source: () => { + if (res.status() === 'error') { + return ERROR; + } + return res.value(); + }, + computation: (source, previous) => { + if (source === ERROR) { + if (previous === undefined) { + throw new Error( + 'impossible state: previous value is not available -> resource was initialized with error', + ); + } + return previous.value; + } else { + return source; + } + }, + }); + } +} From 5a9ae26937f8e90b8851f396b613b5e764f2326d Mon Sep 17 00:00:00 2001 From: Rainer Hahnekamp Date: Sun, 28 Dec 2025 03:03:20 +0100 Subject: [PATCH 2/3] feat: improve implementation --- .../src/lib/with-resource.spec.ts | 403 +++++++++--------- libs/ngrx-toolkit/src/lib/with-resource.ts | 56 ++- 2 files changed, 250 insertions(+), 209 deletions(-) diff --git a/libs/ngrx-toolkit/src/lib/with-resource.spec.ts b/libs/ngrx-toolkit/src/lib/with-resource.spec.ts index 3e28049c..c1af6ccc 100644 --- a/libs/ngrx-toolkit/src/lib/with-resource.spec.ts +++ b/libs/ngrx-toolkit/src/lib/with-resource.spec.ts @@ -191,226 +191,237 @@ describe('withResource', () => { }); describe('status checks', () => { - const errorHandlings: ErrorHandling[] = [ + describe.each([ 'native', 'undefined value', 'previous value', - ]; - for (const errorHandling of errorHandlings) { - describe(`Error Handling: ${errorHandling}`, () => { - for (const { name, setup } of [ - { - name: 'unnamed resource', - setup: () => { - const { store, addressResolver } = - setupWithUnnamedResource(errorHandling); - const setId = (id: number) => patchState(store, { id }); - const setValue = (value: Address) => - patchState(store, { value }); - return { - storeAndResource: store, - addressResolver, - setId, - setValue, - }; - }, + ] as ErrorHandling[])(`Error Handling: %s`, (errorHandling) => { + const testParameters = [ + { + name: 'unnamed resource', + setup: () => { + const { store, addressResolver } = + setupWithUnnamedResource(errorHandling); + const setId = (id: number) => patchState(store, { id }); + const setValue = (value: Address) => + patchState(store, { value }); + return { + storeAndResource: store, + addressResolver, + setId, + setValue, + }; }, - { - name: 'mapped named resource', - setup: () => { - const { store, addressResolver } = - setupWithNamedResource(errorHandling); - const storeAndResource = mapToResource(store, 'address'); - const setId = (id: number) => patchState(store, { id }); - const setValue = (value: Address) => - patchState(store, { addressValue: value }); - return { storeAndResource, addressResolver, setId, setValue }; - }, + }, + { + name: 'mapped named resource', + setup: () => { + const { store, addressResolver } = + setupWithNamedResource(errorHandling); + const storeAndResource = mapToResource(store, 'address'); + const setId = (id: number) => patchState(store, { id }); + const setValue = (value: Address) => + patchState(store, { addressValue: value }); + return { storeAndResource, addressResolver, setId, setValue }; }, - ]) { - describe(name, () => { - it('has idle status in the beginning', () => { - const { storeAndResource } = setup(); - - expect(storeAndResource.status()).toBe('idle'); - expect(storeAndResource.value()).toBeUndefined(); - expect(storeAndResource.error()).toBeUndefined(); - expect(storeAndResource.isLoading()).toBe(false); - expect(storeAndResource.hasValue()).toBe(false); - }); - - it('has loading status when loading', () => { - const { storeAndResource, addressResolver, setId } = setup(); - - addressResolver.resolve.mockResolvedValue(venice); - setId(1); - - expect(storeAndResource.status()).toBe('loading'); - expect(storeAndResource.value()).toBeUndefined(); - expect(storeAndResource.error()).toBeUndefined(); - expect(storeAndResource.isLoading()).toBe(true); - expect(storeAndResource.hasValue()).toBe(false); - }); - - it('has resolved status when loaded', async () => { - const { storeAndResource, addressResolver, setId } = setup(); - - addressResolver.resolve.mockResolvedValue(venice); - setId(1); - - await wait(); - - expect(storeAndResource.status()).toBe('resolved'); - expect(storeAndResource.value()).toEqual(venice); - expect(storeAndResource.error()).toBeUndefined(); - expect(storeAndResource.isLoading()).toBe(false); - expect(storeAndResource.hasValue()).toBe(true); - }); - - it('has error status when error', async () => { - const { storeAndResource, addressResolver, setId } = setup(); - - addressResolver.resolve.mockRejectedValue(new Error('Error')); - setId(1); - await wait(); - - expect(storeAndResource.status()).toBe('error'); - expect(() => storeAndResource.value()).toThrow(); - expect(storeAndResource.error()).toBeInstanceOf(Error); - expect(storeAndResource.isLoading()).toBe(false); - expect(storeAndResource.hasValue()).toBe(false); - }); - - it('has local once updated', async () => { - const { storeAndResource, addressResolver, setId, setValue } = - setup(); - - addressResolver.resolve.mockResolvedValue(venice); - setId(1); - - await wait(); - setValue({ ...venice, country: 'Italia' }); - - expect(storeAndResource.status()).toBe('local'); - expect(storeAndResource.value()?.country).toBe('Italia'); - expect(storeAndResource.error()).toBeUndefined(); - expect(storeAndResource.isLoading()).toBe(false); - expect(storeAndResource.hasValue()).toBe(true); - }); - }); - } - - it('reloads an unnamed resource', async () => { - const { store, addressResolver } = - setupWithUnnamedResource(errorHandling); + }, + { + name: 'mapped named resource', + setup: () => { + const { store, addressResolver } = + setupWithNamedResource(errorHandling); + const storeAndResource = mapToResource(store, 'address'); + const setId = (id: number) => patchState(store, { id }); + const setValue = (value: Address) => + patchState(store, { addressValue: value }); + return { storeAndResource, addressResolver, setId, setValue }; + }, + }, + ]; + + describe.each(testParameters)(`$name`, ({ setup }) => { + it('has idle status in the beginning', () => { + const { storeAndResource } = setup(); + + expect(storeAndResource.status()).toBe('idle'); + expect(storeAndResource.value()).toBeUndefined(); + expect(storeAndResource.error()).toBeUndefined(); + expect(storeAndResource.isLoading()).toBe(false); + expect(storeAndResource.hasValue()).toBe(false); + }); + + it('has loading status when loading', () => { + const { storeAndResource, addressResolver, setId } = setup(); addressResolver.resolve.mockResolvedValue(venice); - patchState(store, { id: 1 }); + setId(1); + + expect(storeAndResource.status()).toBe('loading'); + expect(storeAndResource.value()).toBeUndefined(); + expect(storeAndResource.error()).toBeUndefined(); + expect(storeAndResource.isLoading()).toBe(true); + expect(storeAndResource.hasValue()).toBe(false); + }); + + it('has resolved status when loaded', async () => { + const { storeAndResource, addressResolver, setId } = setup(); + + addressResolver.resolve.mockResolvedValue(venice); + setId(1); await wait(); - expect(store.hasValue()).toBe(true); - addressResolver.resolve.mockResolvedValue({ - ...venice, - country: 'Great Britain', - }); - store.reload(); + expect(storeAndResource.status()).toBe('resolved'); + expect(storeAndResource.value()).toEqual(venice); + expect(storeAndResource.error()).toBeUndefined(); + expect(storeAndResource.isLoading()).toBe(false); + expect(storeAndResource.hasValue()).toBe(true); + }); + + it('has local once updated', async () => { + const { storeAndResource, addressResolver, setId, setValue } = + setup(); + + addressResolver.resolve.mockResolvedValue(venice); + setId(1); await wait(); - expect(store.value()?.country).toBe('Great Britain'); + setValue({ ...venice, country: 'Italia' }); + + expect(storeAndResource.status()).toBe('local'); + expect(storeAndResource.value()?.country).toBe('Italia'); + expect(storeAndResource.error()).toBeUndefined(); + expect(storeAndResource.isLoading()).toBe(false); + expect(storeAndResource.hasValue()).toBe(true); }); + }); - describe('named resource', () => { - it('has idle status in the beginning', () => { - const { store } = setupWithNamedResource(errorHandling); + it('reloads an unnamed resource', async () => { + const { store, addressResolver } = + setupWithUnnamedResource(errorHandling); - expect(store.addressStatus()).toBe('idle'); - expect(store.addressValue()).toBeUndefined(); - expect(store.addressError()).toBeUndefined(); - expect(store.addressIsLoading()).toBe(false); - expect(store.addressHasValue()).toBe(false); - }); + addressResolver.resolve.mockResolvedValue(venice); + patchState(store, { id: 1 }); - it('has loading status when loading', () => { - const { store, addressResolver } = - setupWithNamedResource(errorHandling); + await wait(); + expect(store.hasValue()).toBe(true); - addressResolver.resolve.mockResolvedValue(venice); - patchState(store, { id: 1 }); + addressResolver.resolve.mockResolvedValue({ + ...venice, + country: 'Great Britain', + }); + store.reload(); - expect(store.addressStatus()).toBe('loading'); - expect(store.addressValue()).toBeUndefined(); - expect(store.addressError()).toBeUndefined(); - expect(store.addressIsLoading()).toBe(true); - expect(store.addressHasValue()).toBe(false); - }); + await wait(); + expect(store.value()?.country).toBe('Great Britain'); + }); - it('has resolved status when loaded', async () => { - const { store, addressResolver } = - setupWithNamedResource(errorHandling); + describe('named resource', () => { + it('has idle status in the beginning', () => { + const { store } = setupWithNamedResource(errorHandling); - addressResolver.resolve.mockResolvedValue(venice); - patchState(store, { id: 1 }); + expect(store.addressStatus()).toBe('idle'); + expect(store.addressValue()).toBeUndefined(); + expect(store.addressError()).toBeUndefined(); + expect(store.addressIsLoading()).toBe(false); + expect(store.addressHasValue()).toBe(false); + }); - await wait(); + it('has loading status when loading', () => { + const { store, addressResolver } = + setupWithNamedResource(errorHandling); - expect(store.addressStatus()).toBe('resolved'); - expect(store.addressValue()).toEqual(venice); - expect(store.addressError()).toBeUndefined(); - expect(store.addressIsLoading()).toBe(false); - expect(store.addressHasValue()).toBe(true); - }); + addressResolver.resolve.mockResolvedValue(venice); + patchState(store, { id: 1 }); - it('has error status when error', async () => { - const { store, addressResolver } = - setupWithNamedResource(errorHandling); + expect(store.addressStatus()).toBe('loading'); + expect(store.addressValue()).toBeUndefined(); + expect(store.addressError()).toBeUndefined(); + expect(store.addressIsLoading()).toBe(true); + expect(store.addressHasValue()).toBe(false); + }); - addressResolver.resolve.mockRejectedValue(new Error('Error')); - patchState(store, { id: 1 }); - await wait(); + it('has resolved status when loaded', async () => { + const { store, addressResolver } = + setupWithNamedResource(errorHandling); - expect(store.addressStatus()).toBe('error'); - expect(() => store.addressValue()).toThrow(); - expect(store.addressError()).toBeInstanceOf(Error); - expect(store.addressIsLoading()).toBe(false); - expect(store.addressHasValue()).toBe(false); - }); + addressResolver.resolve.mockResolvedValue(venice); + patchState(store, { id: 1 }); - it('has local once updated', async () => { - const { store, addressResolver } = - setupWithNamedResource(errorHandling); + await wait(); - addressResolver.resolve.mockResolvedValue(venice); - patchState(store, { id: 1 }); + expect(store.addressStatus()).toBe('resolved'); + expect(store.addressValue()).toEqual(venice); + expect(store.addressError()).toBeUndefined(); + expect(store.addressIsLoading()).toBe(false); + expect(store.addressHasValue()).toBe(true); + }); - await wait(); - patchState(store, ({ addressValue }) => ({ - addressValue: addressValue - ? { ...addressValue, country: 'Italia' } - : undefined, - })); + it('has error status when error', async () => { + const { store, addressResolver } = + setupWithNamedResource(errorHandling); - expect(store.addressStatus()).toBe('local'); - expect(store.addressValue()?.country).toBe('Italia'); - expect(store.addressError()).toBeUndefined(); - expect(store.addressIsLoading()).toBe(false); - expect(store.addressHasValue()).toBe(true); - }); + addressResolver.resolve.mockRejectedValue(new Error('Error')); + patchState(store, { id: 1 }); + await wait(); - it('can also reload by resource name', async () => { - const { store, addressResolver } = - setupWithNamedResource(errorHandling); + expect(store.addressStatus()).toBe('error'); + expect(() => store.addressValue()).toThrow(); + expect(store.addressError()).toBeInstanceOf(Error); + expect(store.addressIsLoading()).toBe(false); + expect(store.addressHasValue()).toBe(false); + }); - addressResolver.resolve.mockResolvedValueOnce(venice); - patchState(store, { id: 1 }); - await wait(); - expect(store.addressStatus()).toBe('resolved'); - store.reload(); - expect(store.addressStatus()).toBe('reloading'); - }); + it('has local once updated', async () => { + const { store, addressResolver } = + setupWithNamedResource(errorHandling); + + addressResolver.resolve.mockResolvedValue(venice); + patchState(store, { id: 1 }); + + await wait(); + patchState(store, ({ addressValue }) => ({ + addressValue: addressValue + ? { ...addressValue, country: 'Italia' } + : undefined, + })); + + expect(store.addressStatus()).toBe('local'); + expect(store.addressValue()?.country).toBe('Italia'); + expect(store.addressError()).toBeUndefined(); + expect(store.addressIsLoading()).toBe(false); + expect(store.addressHasValue()).toBe(true); + }); + + it('can also reload by resource name', async () => { + const { store, addressResolver } = + setupWithNamedResource(errorHandling); + + addressResolver.resolve.mockResolvedValueOnce(venice); + patchState(store, { id: 1 }); + await wait(); + expect(store.addressStatus()).toBe('resolved'); + store.reload(); + expect(store.addressStatus()).toBe('reloading'); }); }); - } + + describe('error status', () => { + it('has error status when error', async () => { + const { storeAndResource, addressResolver, setId } = setup(); + + addressResolver.resolve.mockRejectedValue(new Error('Error')); + setId(1); + await wait(); + + expect(storeAndResource.status()).toBe('error'); + expect(() => storeAndResource.value()).toThrow(); + expect(storeAndResource.error()).toBeInstanceOf(Error); + expect(storeAndResource.isLoading()).toBe(false); + expect(storeAndResource.hasValue()).toBe(false); + }); + }); + }); }); describe('override protection', () => { @@ -420,16 +431,10 @@ describe('withResource', () => { warningSpy.mockClear(); }); - for (const memberName of [ - //TODO wait for https://github.com/ngrx/platform/pull/4932 - // 'value', - 'status', - 'error', - 'isLoading', - '_reload', - 'hasValue', - ]) { - it(`warns if ${memberName} is not a member of the store`, () => { + //TODO wait for https://github.com/ngrx/platform/pull/4932 and then add 'value' to the list + it.each(['status', 'error', 'isLoading', '_reload', 'hasValue'])( + `warns if %s is not a member of the store`, + (memberName) => { const Store = signalStore( { providedIn: 'root' }, withProps(() => ({ [memberName]: true })), @@ -445,8 +450,8 @@ describe('withResource', () => { 'Trying to override:', memberName, ); - }); - } + }, + ); //TODO wait for https://github.com/ngrx/platform/pull/4932 it.skip('also checks for named resources', () => { diff --git a/libs/ngrx-toolkit/src/lib/with-resource.ts b/libs/ngrx-toolkit/src/lib/with-resource.ts index fd54fd6c..be1cda3f 100644 --- a/libs/ngrx-toolkit/src/lib/with-resource.ts +++ b/libs/ngrx-toolkit/src/lib/with-resource.ts @@ -34,10 +34,17 @@ export type ResourceResult = { export type ResourceDictionary = Record>; -export type NamedResourceResult = { +export type NamedResourceResult< + T extends ResourceDictionary, + UndefinedErrorHandling extends boolean, +> = { state: { [Prop in keyof T as `${Prop & - string}Value`]: T[Prop]['value'] extends Signal ? S : never; + string}Value`]: T[Prop]['value'] extends Signal + ? UndefinedErrorHandling extends true + ? S | undefined + : S + : never; }; props: { [Prop in keyof T as `${Prop & string}Status`]: Signal; @@ -74,7 +81,7 @@ 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 + * The resource's value is stored under the `value` key in the state * and is exposed as a `DeepSignal`. * * It can also be updated via `patchState`. @@ -96,8 +103,19 @@ const defaultOptions: Required = { * ``` * * @param resourceFactory A factory function that receives the store's state signals, - * methods, and props. Needs to return a `ResourceRef`. + * methods, and props. + * @param resourceOptions Allows to configure the error handling behavior. */ +export function withResource< + Input extends SignalStoreFeatureResult, + ResourceValue, +>( + resourceFactory: ( + store: Input['props'] & Input['methods'] & StateSignals, + ) => ResourceRef, + resourceOptions: { errorHandling: 'undefined value' }, +): SignalStoreFeature>; + export function withResource< Input extends SignalStoreFeatureResult, ResourceValue, @@ -140,7 +158,18 @@ export function withResource< * * @param resourceFactory A factory function that receives the store's props, * methods, and state signals. It must return a `Record`. + * @param resourceOptions Allows to configure the error handling behavior. */ +export function withResource< + Input extends SignalStoreFeatureResult, + Dictionary extends ResourceDictionary, +>( + resourceFactory: ( + store: Input['props'] & Input['methods'] & StateSignals, + ) => Dictionary, + resourceOptions: { errorHandling: 'undefined value' }, +): SignalStoreFeature>; + export function withResource< Input extends SignalStoreFeatureResult, Dictionary extends ResourceDictionary, @@ -149,7 +178,7 @@ export function withResource< store: Input['props'] & Input['methods'] & StateSignals, ) => Dictionary, resourceOptions?: ResourceOptions, -): SignalStoreFeature>; +): SignalStoreFeature>; export function withResource< Input extends SignalStoreFeatureResult, @@ -195,10 +224,7 @@ function createUnnamedResource( return signalStoreFeature( withLinkedState(() => ({ - value: valueSignalForErrorHandling( - resource, - errorHandling, - ) as WritableSignal, + value: valueSignalForErrorHandling(resource, errorHandling), })), withProps(() => ({ status: resource.status, @@ -362,10 +388,20 @@ const ERROR = Symbol('ERROR'); // Tests need to check if local changes are also executed. // Test needs to check if we go directly from initial into value. what happens if default value is there, what happens if undefined. +function valueSignalForErrorHandling( + res: ResourceRef, + errorHandling: 'undefined value', +): WritableSignal; + +function valueSignalForErrorHandling( + res: ResourceRef, + errorHandling: ErrorHandling, +): WritableSignal; + function valueSignalForErrorHandling( res: ResourceRef, errorHandling: ErrorHandling, -): WritableSignal { +): WritableSignal { switch (errorHandling) { case 'native': return res.value; From 94d370c8e8cb440b33c6a3c82e67808932320852 Mon Sep 17 00:00:00 2001 From: Rainer Hahnekamp Date: Sun, 28 Dec 2025 17:36:20 +0100 Subject: [PATCH 3/3] feat: add description about issues and two approaches --- .../src/lib/with-resource.spec.ts | 26 +++++--- libs/ngrx-toolkit/src/lib/with-resource.ts | 61 ++++++++++++++++++- 2 files changed, 76 insertions(+), 11 deletions(-) diff --git a/libs/ngrx-toolkit/src/lib/with-resource.spec.ts b/libs/ngrx-toolkit/src/lib/with-resource.spec.ts index c1af6ccc..6c0aaa02 100644 --- a/libs/ngrx-toolkit/src/lib/with-resource.spec.ts +++ b/libs/ngrx-toolkit/src/lib/with-resource.spec.ts @@ -405,22 +405,30 @@ describe('withResource', () => { expect(store.addressStatus()).toBe('reloading'); }); }); + }); - describe('error status', () => { - it('has error status when error', async () => { - const { storeAndResource, addressResolver, setId } = setup(); + describe('error status', () => { + describe('unnamed resource', () => { + it('throws on error with native error handling', async () => { + const { store, addressResolver } = + setupWithUnnamedResource('native'); addressResolver.resolve.mockRejectedValue(new Error('Error')); - setId(1); + patchState(store, { id: 1 }); await wait(); - expect(storeAndResource.status()).toBe('error'); - expect(() => storeAndResource.value()).toThrow(); - expect(storeAndResource.error()).toBeInstanceOf(Error); - expect(storeAndResource.isLoading()).toBe(false); - expect(storeAndResource.hasValue()).toBe(false); + expect(() => store.value()).toThrow(); }); }); + it('returns undefined with undefined value error handling', async () => { + const { store, addressResolver } = + setupWithUnnamedResource('undefined value'); + + addressResolver.resolve.mockRejectedValue(new Error('Error')); + patchState(store, { id: 1 }); + await wait(); + expect(store.value()).toBeUndefined(); + }); }); }); diff --git a/libs/ngrx-toolkit/src/lib/with-resource.ts b/libs/ngrx-toolkit/src/lib/with-resource.ts index be1cda3f..c81d3198 100644 --- a/libs/ngrx-toolkit/src/lib/with-resource.ts +++ b/libs/ngrx-toolkit/src/lib/with-resource.ts @@ -380,14 +380,71 @@ export function mapToResource< } as MappedResource; } +/** + * Strategies to work around the error throwing behavior of the Resource API. + * + * The original idea was to use a `linkedSignal` as the state's value Signal. It would mean + * that we can leverage the `computation` callback to handle the error. The downside is that + * changes to that signal will not be reflected in the underlying resource, i.e. the resource + * will not switch to status `local`. + * + * 1. An option to fix that would be to put the `linkedSignal` as property in the SignalStore, + * where it would have the name `value`. Given, we apply a `DeepSignal` to it, it would not + * break from the outside. The original value would be put into the state behind a hidden Symbol + * as property name. In order to update the state, users will get an updater function, called + * `setResource`. + * + * That works perfectly for unnamed resources, but could cause potential problems + * for named resources, when they are defined multiple times, i.e. calling `withResource` + * multiple times. The reason is that, we would have to hide their values in the state again + * behind a symbol, but that would be a property which gets defined once, and would get new + * subproperties (the values of the resources) added per additional `withResource` call. + * + * Using a separate updated method is a common SignalStore pattern, which is also used + * in `withEntities`. + * + * For named resources, `setResource` would come with a name as first parameter. + * + * We saw in earlier experiments that there are TypeScript-specific challenges. + * + * Pros: + * - Uses Angular's native `linkedSignal` and isn't a hackish approach + * - Status transitions to 'local' work correctly (via direct `res.value.set()` in `setResource`) + * - Works with `patchState`/`getState` (linkedSignal handles errors on read) + * - Clear, explicit API with dedicated `setResource()` method + * + * Cons: + * - Requires API change: users must use `setResource()` instead of `patchState(store, { value })` + * - Named resources with multiple `withResource` calls: hidden state management becomes complex + * + * 2. A possible alternative would be to use a Proxy on value. Instead of using a `linkedSignal`, + * we can leave the value signal as is and create a proxy around it that intercepts the get/call + * operation and handles the error. The downside is that we need to implement the proxy ourselves, + * which is not as clean as using a `linkedSignal`. On the other hand, the Angular team is working + * on a better way to handle errors, which means that approach is only temporary. It could also + * happen, that we are getting some sort of "Mapped Signal", where not just the reading (as in + * `linkedSignal`) but also the writing is handled. + * + * Pros: + * - No API changes: `patchState(store, { value: x })` works naturally + * - Status transitions to 'local' work correctly (writes go directly to original signal) + * - Works with `patchState`/`getState` (proxy intercepts reads and handles errors) + * - Uniform solution: same approach for both named and unnamed resources + * - Transparent: looks and behaves like a normal signal from the outside + * + * Cons: + * - Manual implementation: must properly handle all signal methods (`set`, `update`, `asReadonly`, etc.) + * - Dependency tracking: need to verify proxy doesn't break Angular's reactivity system + * - More complex proxy logic required for 'previous value' strategy (caching previous values) + * - Less "Angular-native": doesn't leverage `linkedSignal`'s built-in reactivity guarantees + */ + // We require an explicit error symbol, to clearly communicate // that an error has happened. Since a resource's value can be // any value, we're ensuring here that an error of true is not // mistakenly seen as value. const ERROR = Symbol('ERROR'); -// Tests need to check if local changes are also executed. -// Test needs to check if we go directly from initial into value. what happens if default value is there, what happens if undefined. function valueSignalForErrorHandling( res: ResourceRef, errorHandling: 'undefined value',