Skip to content

Commit 906413a

Browse files
alxhubAndrewKushnir
alxhub
authored and
AndrewKushnir
committed
fix(core): change Resource to use explicit undefined in its typings (#59024)
Originally the `T` in `Resource<T>` represented the resolved type of the resource, and `undefined` was explicitly added to this type in the `.value` signal. This turned out to be problematic, as it wasn't possible to write a type for a resource which didn't return `undefined` values. Such a type is useful for 2 reasons: 1. to support narrowing of the resource type when `Resource.hasValue()` returns `true`. 2. for resources which use a different value instead of `undefined` to represent not having a value (for example, array resources which want to use `[]` as their default). Instead, this commit changes `resource()` and `rxResource()` to return an explicit `ResourceRef<T|undefined>`, and removes the union with `undefined` from all types related to the resource's value. This way, it's trivially possible to write `Resource<T>` to represent resources where `.value` only returns `T`. `hasValue()` then actually works to perform narrowing, by narrowing the resource type to `Exclude<T, undefined>`. PR Close #59024
1 parent d12a186 commit 906413a

File tree

5 files changed

+40
-41
lines changed

5 files changed

+40
-41
lines changed

‎goldens/public-api/core/index.api.md‎

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1585,17 +1585,15 @@ export function resolveForwardRef<T>(type: T): T;
15851585
// @public
15861586
export interface Resource<T> {
15871587
readonly error: Signal<unknown>;
1588-
hasValue(): this is Resource<T> & {
1589-
value: Signal<T>;
1590-
};
1588+
hasValue(): this is Resource<Exclude<T, undefined>>;
15911589
readonly isLoading: Signal<boolean>;
15921590
reload(): boolean;
15931591
readonly status: Signal<ResourceStatus>;
1594-
readonly value: Signal<T | undefined>;
1592+
readonly value: Signal<T>;
15951593
}
15961594

15971595
// @public
1598-
export function resource<T, R>(options: ResourceOptions<T, R>): ResourceRef<T>;
1596+
export function resource<T, R>(options: ResourceOptions<T, R>): ResourceRef<T | undefined>;
15991597

16001598
// @public
16011599
export type ResourceLoader<T, R> = (param: ResourceLoaderParams<R>) => PromiseLike<T>;
@@ -1984,13 +1982,11 @@ export interface WritableResource<T> extends Resource<T> {
19841982
// (undocumented)
19851983
asReadonly(): Resource<T>;
19861984
// (undocumented)
1987-
hasValue(): this is WritableResource<T> & {
1988-
value: WritableSignal<T>;
1989-
};
1990-
set(value: T | undefined): void;
1991-
update(updater: (value: T | undefined) => T | undefined): void;
1985+
hasValue(): this is WritableResource<Exclude<T, undefined>>;
1986+
set(value: T): void;
1987+
update(updater: (value: T) => T): void;
19921988
// (undocumented)
1993-
readonly value: WritableSignal<T | undefined>;
1989+
readonly value: WritableSignal<T>;
19941990
}
19951991

19961992
// @public

‎goldens/public-api/core/rxjs-interop/index.api.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export function outputToObservable<T>(ref: OutputRef<T>): Observable<T>;
2727
export function pendingUntilEvent<T>(injector?: Injector): MonoTypeOperatorFunction<T>;
2828

2929
// @public
30-
export function rxResource<T, R>(opts: RxResourceOptions<T, R>): ResourceRef<T>;
30+
export function rxResource<T, R>(opts: RxResourceOptions<T, R>): ResourceRef<T | undefined>;
3131

3232
// @public
3333
export interface RxResourceOptions<T, R> extends Omit<ResourceOptions<T, R>, 'loader'> {

‎packages/core/rxjs-interop/src/rx_resource.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ export interface RxResourceOptions<T, R> extends Omit<ResourceOptions<T, R>, 'lo
3131
*
3232
* @experimental
3333
*/
34-
export function rxResource<T, R>(opts: RxResourceOptions<T, R>): ResourceRef<T> {
34+
export function rxResource<T, R>(opts: RxResourceOptions<T, R>): ResourceRef<T | undefined> {
3535
opts?.injector || assertInInjectionContext(rxResource);
3636
return resource<T, R>({
3737
...opts,

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ export interface Resource<T> {
6868
/**
6969
* The current value of the `Resource`, or `undefined` if there is no current value.
7070
*/
71-
readonly value: Signal<T | undefined>;
71+
readonly value: Signal<T>;
7272

7373
/**
7474
* The current status of the `Resource`, which describes what the resource is currently doing and
@@ -91,7 +91,7 @@ export interface Resource<T> {
9191
*
9292
* This function is reactive.
9393
*/
94-
hasValue(): this is Resource<T> & {value: Signal<T>};
94+
hasValue(): this is Resource<Exclude<T, undefined>>;
9595

9696
/**
9797
* Instructs the resource to re-load any asynchronous dependency it may have.
@@ -112,18 +112,18 @@ export interface Resource<T> {
112112
* @experimental
113113
*/
114114
export interface WritableResource<T> extends Resource<T> {
115-
readonly value: WritableSignal<T | undefined>;
116-
hasValue(): this is WritableResource<T> & {value: WritableSignal<T>};
115+
readonly value: WritableSignal<T>;
116+
hasValue(): this is WritableResource<Exclude<T, undefined>>;
117117

118118
/**
119119
* Convenience wrapper for `value.set`.
120120
*/
121-
set(value: T | undefined): void;
121+
set(value: T): void;
122122

123123
/**
124124
* Convenience wrapper for `value.update`.
125125
*/
126-
update(updater: (value: T | undefined) => T | undefined): void;
126+
update(updater: (value: T) => T): void;
127127
asReadonly(): Resource<T>;
128128
}
129129

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

Lines changed: 25 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,16 @@ import {DestroyRef} from '../linker';
3636
*
3737
* @experimental
3838
*/
39-
export function resource<T, R>(options: ResourceOptions<T, R>): ResourceRef<T> {
39+
export function resource<T, R>(options: ResourceOptions<T, R>): ResourceRef<T | undefined> {
4040
options?.injector || assertInInjectionContext(resource);
4141
const request = (options.request ?? (() => null)) as () => R;
42-
return new WritableResourceImpl<T, R>(request, options.loader, options.equal, options.injector);
42+
return new WritableResourceImpl<T | undefined, R>(
43+
request,
44+
options.loader,
45+
undefined,
46+
options.equal ? wrapEqualityFn(options.equal) : undefined,
47+
options.injector,
48+
);
4349
}
4450

4551
/**
@@ -49,23 +55,23 @@ export function resource<T, R>(options: ResourceOptions<T, R>): ResourceRef<T> {
4955
* Mainly factored out for better readability.
5056
*/
5157
abstract class BaseWritableResource<T> implements WritableResource<T> {
52-
readonly value: WritableSignal<T | undefined>;
58+
readonly value: WritableSignal<T>;
5359
readonly status = signal<ResourceStatus>(ResourceStatus.Idle);
5460
readonly error = signal<unknown>(undefined);
5561

56-
protected readonly rawSetValue: (value: T | undefined) => void;
62+
protected readonly rawSetValue: (value: T) => void;
5763

58-
constructor(equal: ValueEqualityFn<T> | undefined) {
59-
this.value = signal<T | undefined>(undefined, {
60-
equal: equal ? wrapEqualityFn(equal) : undefined,
61-
});
64+
constructor(
65+
protected defaultValue: T,
66+
equal: ValueEqualityFn<T> | undefined,
67+
) {
68+
this.value = signal<T>(this.defaultValue, {equal});
6269
this.rawSetValue = this.value.set;
63-
this.value.set = (value: T | undefined) => this.set(value);
64-
this.value.update = (fn: (value: T | undefined) => T | undefined) =>
65-
this.set(fn(untracked(this.value)));
70+
this.value.set = (value: T) => this.set(value);
71+
this.value.update = (fn: (value: T) => T) => this.set(fn(untracked(this.value)));
6672
}
6773

68-
set(value: T | undefined): void {
74+
set(value: T): void {
6975
// Set the value signal and check whether its `version` changes. This will tell us
7076
// if the value signal actually updated or not.
7177
const prevVersion = (this.value[SIGNAL] as SignalNode<T>).version;
@@ -82,20 +88,16 @@ abstract class BaseWritableResource<T> implements WritableResource<T> {
8288
this.error.set(undefined);
8389
}
8490

85-
update(updater: (value: T | undefined) => T | undefined): void {
91+
update(updater: (value: T) => T): void {
8692
this.value.update(updater);
8793
}
8894

8995
readonly isLoading = computed(
9096
() => this.status() === ResourceStatus.Loading || this.status() === ResourceStatus.Reloading,
9197
);
9298

93-
hasValue(): this is WritableResource<T> & {value: WritableSignal<T>} {
94-
return (
95-
this.status() === ResourceStatus.Resolved ||
96-
this.status() === ResourceStatus.Local ||
97-
this.status() === ResourceStatus.Reloading
98-
);
99+
hasValue(): this is WritableResource<Exclude<T, undefined>> {
100+
return this.value() !== undefined;
99101
}
100102

101103
asReadonly(): Resource<T> {
@@ -105,7 +107,7 @@ abstract class BaseWritableResource<T> implements WritableResource<T> {
105107
/**
106108
* Put the resource in a state with a given value.
107109
*/
108-
protected setValueState(status: ResourceStatus, value: T | undefined = undefined): void {
110+
protected setValueState(status: ResourceStatus, value: T = this.defaultValue): void {
109111
this.status.set(status);
110112
this.rawSetValue(value);
111113
this.error.set(undefined);
@@ -115,7 +117,7 @@ abstract class BaseWritableResource<T> implements WritableResource<T> {
115117
* Put the resource into the error state.
116118
*/
117119
protected setErrorState(err: unknown): void {
118-
this.value.set(undefined);
120+
this.value.set(this.defaultValue);
119121
// The previous line will set the status to `Local`, so we need to update it.
120122
this.status.set(ResourceStatus.Error);
121123
this.error.set(err);
@@ -142,10 +144,11 @@ class WritableResourceImpl<T, R> extends BaseWritableResource<T> implements Reso
142144
constructor(
143145
requestFn: () => R,
144146
private readonly loaderFn: ResourceLoader<T, R>,
147+
defaultValue: T,
145148
equal: ValueEqualityFn<T> | undefined,
146149
injector: Injector | undefined,
147150
) {
148-
super(equal);
151+
super(defaultValue, equal);
149152
injector = injector ?? inject(Injector);
150153
this.pendingTasks = injector.get(PendingTasks);
151154

0 commit comments

Comments
 (0)