Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Commit a0cd72f

Browse filesBrowse files
HumberdHumberd
committed
fix(core): getting resource value throws an error instead of returning undefined
When there is an underlying error state it would not be possible to swallow the error with: `computed(() => res.value()?.inner);` BREAKING CHANGE: * `Resource.value()` now throws an error when it's in an error state. Previously it returned `undefined`.
1 parent 3ac094a commit a0cd72f
Copy full SHA for a0cd72f

File tree

Expand file treeCollapse file tree

3 files changed

+94
-9
lines changed
Filter options
Expand file treeCollapse file tree

3 files changed

+94
-9
lines changed

‎packages/core/src/resource/api.ts

Copy file name to clipboardExpand all lines: packages/core/src/resource/api.ts
+2-2Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ export type ResourceStatus = 'idle' | 'error' | 'loading' | 'reloading' | 'resol
4646
*/
4747
export interface Resource<T> {
4848
/**
49-
* The current value of the `Resource`, or `undefined` if there is no current value.
49+
* The current value of the `Resource`, or throws an error if the resource is in an error state.
5050
*/
5151
readonly value: Signal<T>;
5252

@@ -167,7 +167,7 @@ export interface BaseResourceOptions<T, R> {
167167

168168
/**
169169
* The value which will be returned from the resource when a server value is unavailable, such as
170-
* when the resource is still loading, or in an error state.
170+
* when the resource is still loading.
171171
*/
172172
defaultValue?: NoInfer<T>;
173173

‎packages/core/src/resource/resource.ts

Copy file name to clipboardExpand all lines: packages/core/src/resource/resource.ts
+16-3Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,10 @@ abstract class BaseWritableResource<T> implements WritableResource<T> {
118118
readonly isLoading = computed(() => this.status() === 'loading' || this.status() === 'reloading');
119119

120120
hasValue(): this is ResourceRef<Exclude<T, undefined>> {
121+
if (this.status() === 'error') {
122+
return false;
123+
}
124+
121125
return this.value() !== undefined;
122126
}
123127

@@ -151,7 +155,7 @@ export class ResourceImpl<T, R> extends BaseWritableResource<T> implements Resou
151155
constructor(
152156
request: () => R,
153157
private readonly loaderFn: ResourceStreamingLoader<T, R>,
154-
private readonly defaultValue: T,
158+
defaultValue: T,
155159
private readonly equal: ValueEqualityFn<T> | undefined,
156160
injector: Injector,
157161
) {
@@ -161,7 +165,16 @@ export class ResourceImpl<T, R> extends BaseWritableResource<T> implements Resou
161165
computed(
162166
() => {
163167
const streamValue = this.state().stream?.();
164-
return streamValue && isResolved(streamValue) ? streamValue.value : this.defaultValue;
168+
169+
if (!streamValue) {
170+
return defaultValue;
171+
}
172+
173+
if (!isResolved(streamValue)) {
174+
throw new Error('Resource is currently in the error state', {cause: this.error()});
175+
}
176+
177+
return streamValue.value;
165178
},
166179
{equal},
167180
),
@@ -398,7 +411,7 @@ function projectStatusOfState(state: ResourceState<unknown>): ResourceStatus {
398411
case 'loading':
399412
return state.extRequest.reload === 0 ? 'loading' : 'reloading';
400413
case 'resolved':
401-
return isResolved(untracked(state.stream!)) ? 'resolved' : 'error';
414+
return isResolved(state.stream!()) ? 'resolved' : 'error';
402415
default:
403416
return state.status;
404417
}

‎packages/core/test/resource/resource_spec.ts

Copy file name to clipboardExpand all lines: packages/core/test/resource/resource_spec.ts
+76-4Lines changed: 76 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88

99
import {
1010
ApplicationRef,
11+
computed,
1112
createEnvironmentInjector,
1213
EnvironmentInjector,
1314
Injector,
@@ -142,7 +143,11 @@ describe('resource', () => {
142143
expect(echoResource.status()).toBe('error');
143144
expect(echoResource.isLoading()).toBeFalse();
144145
expect(echoResource.hasValue()).toBeFalse();
145-
expect(echoResource.value()).toEqual(undefined);
146+
expect(() => echoResource.value()).toThrow(
147+
new Error('Resource is currently in the error state', {
148+
cause: new Error('Unknown error', {cause: 'Something went wrong....'}),
149+
}),
150+
);
146151
expect(echoResource.error()).toEqual(
147152
new Error('Unknown error', {cause: 'Something went wrong....'}),
148153
);
@@ -179,8 +184,73 @@ describe('resource', () => {
179184
expect(echoResource.status()).toBe('error');
180185
expect(echoResource.isLoading()).toBeFalse();
181186
expect(echoResource.hasValue()).toBeFalse();
182-
expect(echoResource.value()).toEqual(undefined);
187+
expect(() => echoResource.value()).toThrow(
188+
new Error('Resource is currently in the error state', {cause: new Error('KO')}),
189+
);
183190
expect(echoResource.error()).toEqual(Error('KO'));
191+
192+
counter.update((value) => value + 1);
193+
TestBed.tick();
194+
await backend.flush();
195+
196+
expect(echoResource.status()).toBe('resolved');
197+
expect(echoResource.isLoading()).toBeFalse();
198+
expect(echoResource.hasValue()).toBeTrue();
199+
expect(echoResource.value()).toEqual('ok');
200+
expect(echoResource.error()).toBe(undefined);
201+
});
202+
203+
it('should update computed signals', async () => {
204+
const backend = new MockEchoBackend();
205+
const counter = signal(0);
206+
const echoResource = resource({
207+
params: () => ({counter: counter()}),
208+
loader: (params) => {
209+
if (params.params.counter % 2 === 0) {
210+
return Promise.resolve(params.params.counter);
211+
} else {
212+
throw new Error('KO');
213+
}
214+
},
215+
injector: TestBed.inject(Injector),
216+
});
217+
const computedValue = computed(() => {
218+
if (!echoResource.hasValue()) {
219+
return -1;
220+
}
221+
return echoResource.value();
222+
});
223+
224+
TestBed.tick();
225+
await backend.flush();
226+
227+
expect(echoResource.status()).toBe('resolved');
228+
expect(echoResource.hasValue()).toBeTrue();
229+
expect(echoResource.value()).toEqual(0);
230+
expect(computedValue()).toEqual(0);
231+
expect(echoResource.error()).toBe(undefined);
232+
233+
counter.update((value) => value + 1);
234+
TestBed.tick();
235+
await backend.flush();
236+
237+
expect(echoResource.status()).toBe('error');
238+
expect(echoResource.hasValue()).toBeFalse();
239+
expect(() => echoResource.value()).toThrow(
240+
new Error('Resource is currently in the error state', {cause: new Error('KO')}),
241+
);
242+
expect(computedValue()).toEqual(-1);
243+
expect(echoResource.error()).toEqual(Error('KO'));
244+
245+
counter.update((value) => value + 1);
246+
TestBed.tick();
247+
await backend.flush();
248+
249+
expect(echoResource.status()).toBe('resolved');
250+
expect(echoResource.hasValue()).toBeTrue();
251+
expect(echoResource.value()).toEqual(2);
252+
expect(computedValue()).toEqual(2);
253+
expect(echoResource.error()).toBe(undefined);
184254
});
185255

186256
it('should respond to a request that changes while loading', async () => {
@@ -227,7 +297,7 @@ describe('resource', () => {
227297
expect(res.value()).toBe(1);
228298
});
229299

230-
it('should return a default value if provided', async () => {
300+
it('should throw an error when getting a value even when provided with a default value', async () => {
231301
const DEFAULT: string[] = [];
232302
const request = signal(0);
233303
const res = resource({
@@ -252,7 +322,9 @@ describe('resource', () => {
252322
request.set(2);
253323
await TestBed.inject(ApplicationRef).whenStable();
254324
expect(res.error()).not.toBeUndefined();
255-
expect(res.value()).toBe(DEFAULT);
325+
expect(() => res.value()).toThrow(
326+
new Error('Resource is currently in the error state', {cause: new Error('err')}),
327+
);
256328
});
257329

258330
it('should _not_ load if the request resolves to undefined', () => {

0 commit comments

Comments
0 (0)
Morty Proxy This is a proxified and sanitized view of the page, visit original site.