Skip to content

Commit 9515a70

Browse files
JoostKthePunderWoman
JoostK
authored and
thePunderWoman
committed
fix(core): fix narrowing of Resource.hasValue() (#63994)
This commit changes `Resource.hasValue()` and its derived types to improve narrowing of resources whose generic type either does not include `undefined` (i.e. when a default value has been provided) or when the generic type is `unknown`. This fixes the undesirable behavior where `hasValue()` would cause the `else` branch of an `hasValue()` conditional to have a narrowed type of `never`, given that the `hasValue()`'s type guard covers the entire type range already (meaning that the type in the else-branch cannot be inhabited in the type system, yielding the `never` type). By making the `hasValue()` method only a type guard when the generic type includes `undefined` these problems are avoided. Fixes #60766 Fixes #63545 Fixes #63982 PR Close #63994
1 parent 388d25e commit 9515a70

File tree

6 files changed

+150
-8
lines changed

6 files changed

+150
-8
lines changed

‎goldens/public-api/common/http/index.api.md‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2894,7 +2894,9 @@ export interface HttpResourceRef<T> extends WritableResource<T>, ResourceRef<T>
28942894
// (undocumented)
28952895
destroy(): void;
28962896
// (undocumented)
2897-
hasValue(): this is HttpResourceRef<Exclude<T, undefined>>;
2897+
hasValue(this: T extends undefined ? this : never): this is HttpResourceRef<Exclude<T, undefined>>;
2898+
// (undocumented)
2899+
hasValue(): boolean;
28982900
readonly headers: Signal<HttpHeaders | undefined>;
28992901
readonly progress: Signal<HttpProgressEvent | undefined>;
29002902
readonly statusCode: Signal<number | undefined>;

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

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1626,7 +1626,9 @@ export function resolveForwardRef<T>(type: T): T;
16261626
// @public
16271627
export interface Resource<T> {
16281628
readonly error: Signal<Error | undefined>;
1629-
hasValue(): this is Resource<Exclude<T, undefined>>;
1629+
hasValue(this: T extends undefined ? this : never): this is Resource<Exclude<T, undefined>>;
1630+
// (undocumented)
1631+
hasValue(): boolean;
16301632
readonly isLoading: Signal<boolean>;
16311633
readonly status: Signal<ResourceStatus>;
16321634
readonly value: Signal<T>;
@@ -1662,7 +1664,9 @@ export type ResourceOptions<T, R> = PromiseResourceOptions<T, R> | StreamingReso
16621664
export interface ResourceRef<T> extends WritableResource<T> {
16631665
destroy(): void;
16641666
// (undocumented)
1665-
hasValue(): this is ResourceRef<Exclude<T, undefined>>;
1667+
hasValue(this: T extends undefined ? this : never): this is ResourceRef<Exclude<T, undefined>>;
1668+
// (undocumented)
1669+
hasValue(): boolean;
16661670
}
16671671

16681672
// @public
@@ -2038,7 +2042,9 @@ export interface WritableResource<T> extends Resource<T> {
20382042
// (undocumented)
20392043
asReadonly(): Resource<T>;
20402044
// (undocumented)
2041-
hasValue(): this is WritableResource<Exclude<T, undefined>>;
2045+
hasValue(this: T extends undefined ? this : never): this is WritableResource<Exclude<T, undefined>>;
2046+
// (undocumented)
2047+
hasValue(): boolean;
20422048
reload(): boolean;
20432049
set(value: T): void;
20442050
update(updater: (value: T) => T): void;

‎packages/common/http/src/resource_api.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,11 @@ export interface HttpResourceRef<T> extends WritableResource<T>, ResourceRef<T>
191191
*/
192192
readonly progress: Signal<HttpProgressEvent | undefined>;
193193

194-
hasValue(): this is HttpResourceRef<Exclude<T, undefined>>;
194+
hasValue(
195+
this: T extends undefined ? this : never,
196+
): this is HttpResourceRef<Exclude<T, undefined>>;
197+
198+
hasValue(): boolean;
199+
195200
destroy(): void;
196201
}

‎packages/common/http/test/resource_spec.ts‎

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import {
1515
httpResource,
1616
HttpContext,
1717
HttpContextToken,
18+
HttpResourceRef,
1819
} from '../index';
1920
import {HttpTestingController, provideHttpClientTesting} from '../testing';
2021

@@ -338,4 +339,49 @@ describe('httpResource', () => {
338339
expect(res.progress()).toBe(undefined);
339340
expect(res.statusCode()).toBe(undefined);
340341
});
342+
343+
describe('types', () => {
344+
it('should narrow hasValue() when the value can be undefined', () => {
345+
const result: HttpResourceRef<number | undefined> = httpResource(() => '/data', {
346+
injector: TestBed.inject(Injector),
347+
parse: () => 0,
348+
});
349+
350+
if (result.hasValue()) {
351+
const _value: number = result.value();
352+
} else if (result.isLoading()) {
353+
// @ts-expect-error
354+
const _value: number = result.value();
355+
} else if (result.error()) {
356+
}
357+
});
358+
359+
it('should not narrow hasValue() when a default value is provided', () => {
360+
const result: HttpResourceRef<number> = httpResource(() => '/data', {
361+
injector: TestBed.inject(Injector),
362+
parse: () => 0,
363+
defaultValue: 0,
364+
});
365+
366+
if (result.hasValue()) {
367+
const _value: number = result.value();
368+
} else if (result.isLoading()) {
369+
const _value: number = result.value();
370+
} else if (result.error()) {
371+
}
372+
});
373+
374+
it('should not narrow hasValue() when the resource type is unknown', () => {
375+
const result: HttpResourceRef<unknown> = httpResource(() => '/data', {
376+
injector: TestBed.inject(Injector),
377+
});
378+
379+
if (result.hasValue()) {
380+
const _value: unknown = result.value();
381+
} else if (result.isLoading()) {
382+
const _value: unknown = result.value();
383+
} else if (result.error()) {
384+
}
385+
});
386+
});
341387
});

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

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,9 @@ export interface Resource<T> {
7171
*
7272
* This function is reactive.
7373
*/
74-
hasValue(): this is Resource<Exclude<T, undefined>>;
74+
hasValue(this: T extends undefined ? this : never): this is Resource<Exclude<T, undefined>>;
75+
76+
hasValue(): boolean;
7577
}
7678

7779
/**
@@ -83,7 +85,11 @@ export interface Resource<T> {
8385
*/
8486
export interface WritableResource<T> extends Resource<T> {
8587
readonly value: WritableSignal<T>;
86-
hasValue(): this is WritableResource<Exclude<T, undefined>>;
88+
hasValue(
89+
this: T extends undefined ? this : never,
90+
): this is WritableResource<Exclude<T, undefined>>;
91+
92+
hasValue(): boolean;
8793

8894
/**
8995
* Convenience wrapper for `value.set`.
@@ -113,8 +119,9 @@ export interface WritableResource<T> extends Resource<T> {
113119
* @experimental
114120
*/
115121
export interface ResourceRef<T> extends WritableResource<T> {
116-
hasValue(): this is ResourceRef<Exclude<T, undefined>>;
122+
hasValue(this: T extends undefined ? this : never): this is ResourceRef<Exclude<T, undefined>>;
117123

124+
hasValue(): boolean;
118125
/**
119126
* Manually destroy the resource, which cancels pending requests and returns it to `idle` state.
120127
*/

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

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import {
1414
EnvironmentInjector,
1515
Injector,
1616
resource,
17+
ResourceRef,
1718
ResourceStatus,
1819
signal,
1920
} from '../../src/core';
@@ -941,6 +942,81 @@ describe('resource', () => {
941942
expect(echoResource.value()).toEqual({});
942943
expect(echoResource.error()).toEqual(undefined);
943944
});
945+
946+
describe('types', () => {
947+
it('should narrow hasValue() when the value can be undefined', () => {
948+
const result: ResourceRef<number | undefined> = resource({
949+
params: () => 1,
950+
loader: async ({params}) => params,
951+
injector: TestBed.inject(Injector),
952+
});
953+
954+
if (result.hasValue()) {
955+
const _value: number = result.value();
956+
} else if (result.isLoading()) {
957+
// @ts-expect-error
958+
const _value: number = result.value();
959+
} else if (result.error()) {
960+
}
961+
962+
const readonly = result.asReadonly();
963+
if (readonly.hasValue()) {
964+
const _value: number = readonly.value();
965+
} else if (readonly.isLoading()) {
966+
// @ts-expect-error
967+
const _value: number = readonly.value();
968+
} else if (readonly.error()) {
969+
}
970+
});
971+
972+
it('should not narrow hasValue() when a default value is provided', () => {
973+
const result: ResourceRef<number> = resource({
974+
params: () => 1,
975+
loader: async ({params}) => params,
976+
injector: TestBed.inject(Injector),
977+
defaultValue: 0,
978+
});
979+
980+
if (result.hasValue()) {
981+
const _value: number = result.value();
982+
} else if (result.isLoading()) {
983+
const _value: number = result.value();
984+
} else if (result.error()) {
985+
}
986+
987+
const readonly = result.asReadonly();
988+
if (readonly.hasValue()) {
989+
const _value: number = readonly.value();
990+
} else if (readonly.isLoading()) {
991+
const _value: number = readonly.value();
992+
} else if (readonly.error()) {
993+
}
994+
});
995+
996+
it('should not narrow hasValue() when the resource type is unknown', () => {
997+
const result: ResourceRef<unknown> = resource({
998+
params: () => 1 as unknown,
999+
loader: async ({params}) => params,
1000+
injector: TestBed.inject(Injector),
1001+
defaultValue: 0,
1002+
});
1003+
1004+
if (result.hasValue()) {
1005+
const _value: unknown = result.value();
1006+
} else if (result.isLoading()) {
1007+
const _value: unknown = result.value();
1008+
} else if (result.error()) {
1009+
}
1010+
1011+
const readonly = result.asReadonly();
1012+
if (readonly.hasValue()) {
1013+
const _value: unknown = readonly.value();
1014+
} else if (readonly.isLoading()) {
1015+
const _value: unknown = readonly.value();
1016+
} else if (readonly.error()) {
1017+
}
1018+
});
1019+
});
9441020
});
9451021

9461022
function flushMicrotasks(): Promise<void> {

0 commit comments

Comments
 (0)