Skip to content

Commit 50d9d55

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 31769e6 commit 50d9d55

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
@@ -2903,7 +2903,9 @@ export interface HttpResourceRef<T> extends WritableResource<T>, ResourceRef<T>
29032903
// (undocumented)
29042904
destroy(): void;
29052905
// (undocumented)
2906-
hasValue(): this is HttpResourceRef<Exclude<T, undefined>>;
2906+
hasValue(this: T extends undefined ? this : never): this is HttpResourceRef<Exclude<T, undefined>>;
2907+
// (undocumented)
2908+
hasValue(): boolean;
29072909
readonly headers: Signal<HttpHeaders | undefined>;
29082910
readonly progress: Signal<HttpProgressEvent | undefined>;
29092911
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
@@ -1600,7 +1600,9 @@ export function resolveForwardRef<T>(type: T): T;
16001600
// @public
16011601
export interface Resource<T> {
16021602
readonly error: Signal<Error | undefined>;
1603-
hasValue(): this is Resource<Exclude<T, undefined>>;
1603+
hasValue(this: T extends undefined ? this : never): this is Resource<Exclude<T, undefined>>;
1604+
// (undocumented)
1605+
hasValue(): boolean;
16041606
readonly isLoading: Signal<boolean>;
16051607
readonly status: Signal<ResourceStatus>;
16061608
readonly value: Signal<T>;
@@ -1636,7 +1638,9 @@ export type ResourceOptions<T, R> = PromiseResourceOptions<T, R> | StreamingReso
16361638
export interface ResourceRef<T> extends WritableResource<T> {
16371639
destroy(): void;
16381640
// (undocumented)
1639-
hasValue(): this is ResourceRef<Exclude<T, undefined>>;
1641+
hasValue(this: T extends undefined ? this : never): this is ResourceRef<Exclude<T, undefined>>;
1642+
// (undocumented)
1643+
hasValue(): boolean;
16401644
}
16411645

16421646
// @public
@@ -2013,7 +2017,9 @@ export interface WritableResource<T> extends Resource<T> {
20132017
// (undocumented)
20142018
asReadonly(): Resource<T>;
20152019
// (undocumented)
2016-
hasValue(): this is WritableResource<Exclude<T, undefined>>;
2020+
hasValue(this: T extends undefined ? this : never): this is WritableResource<Exclude<T, undefined>>;
2021+
// (undocumented)
2022+
hasValue(): boolean;
20172023
reload(): boolean;
20182024
set(value: T): void;
20192025
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)