Skip to content

fix(core): derive possible null value in rxResource stream params (#6… #68171

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
surajy93 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

fix(core): derive possible null value in rxResource stream params (#6… #68171

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 16 additions & 10 deletions goldens/public-api/core/index.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,12 +186,11 @@ export interface AttributeDecorator {
}

// @public
export interface BaseResourceOptions<T, R> {
export type BaseResourceOptions<T, R> = {
defaultValue?: NoInfer<T>;
equal?: ValueEqualityFn<T>;
injector?: Injector;
params?: (ctx: ResourceParamsContext) => R;
}
} & ResourceParams<R>;

// @public
export interface Binding {
Expand Down Expand Up @@ -1444,10 +1443,10 @@ export class PlatformRef {
export type Predicate<T> = (value: T) => boolean;

// @public
export interface PromiseResourceOptions<T, R> extends BaseResourceOptions<T, R> {
export type PromiseResourceOptions<T, R> = BaseResourceOptions<T, R> & {
loader: ResourceLoader<T, R>;
stream?: never;
}
};

// @public
export function provideAppInitializer(initializerFn: () => Observable<unknown> | Promise<unknown> | void): EnvironmentProviders;
Expand Down Expand Up @@ -1624,12 +1623,12 @@ export interface Resource<T> {
}

// @public
export function resource<T, R>(options: ResourceOptions<T, R> & {
export function resource<T, R = null>(options: ResourceOptions<T, R> & {
defaultValue: NoInfer<T>;
}): ResourceRef<T>;

// @public
export function resource<T, R>(options: ResourceOptions<T, R>): ResourceRef<T | undefined>;
export function resource<T, R = null>(options: ResourceOptions<T, R>): ResourceRef<T | undefined>;

// @public
export class ResourceDependencyError extends Error {
Expand Down Expand Up @@ -1660,6 +1659,13 @@ export type ResourceOptions<T, R> = (PromiseResourceOptions<T, R> | StreamingRes
debugName?: string;
};

// @public (undocumented)
export type ResourceParams<R> = [R] extends [null] ? {
params?: (ctx: ResourceParamsContext) => R;
} : {
params: (ctx: ResourceParamsContext) => R;
};

// @public
export interface ResourceParamsContext {
readonly chain: <T>(resource: Resource<T>) => T;
Expand Down Expand Up @@ -1832,10 +1838,10 @@ export interface StaticClassSansProvider {
export type StaticProvider = ValueProvider | ExistingProvider | StaticClassProvider | ConstructorProvider | FactoryProvider | any[];

// @public
export interface StreamingResourceOptions<T, R> extends BaseResourceOptions<T, R> {
loader?: never;
export type StreamingResourceOptions<T, R> = BaseResourceOptions<T, R> & {
stream: ResourceStreamingLoader<T, R>;
}
loader?: never;
};

// @public
export class TemplateRef<C> {
Expand Down
9 changes: 4 additions & 5 deletions goldens/public-api/core/rxjs-interop/index.api.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,18 +19,17 @@ export function outputToObservable<T>(ref: OutputRef<T>): Observable<T>;
export function pendingUntilEvent<T>(injector?: Injector): MonoTypeOperatorFunction<T>;

// @public
export function rxResource<T, R>(opts: RxResourceOptions<T, R> & {
export function rxResource<T, R = null>(opts: RxResourceOptions<T, R> & {
defaultValue: NoInfer<T>;
}): ResourceRef<T>;

// @public
export function rxResource<T, R>(opts: RxResourceOptions<T, R>): ResourceRef<T | undefined>;
export function rxResource<T, R = null>(opts: RxResourceOptions<T, R>): ResourceRef<T | undefined>;

// @public
export interface RxResourceOptions<T, R> extends BaseResourceOptions<T, R> {
// (undocumented)
export type RxResourceOptions<T, R> = BaseResourceOptions<T, R> & {
stream: (params: ResourceLoaderParams<R>) => Observable<T>;
}
};

// @public
export function takeUntilDestroyed<T>(destroyRef?: DestroyRef): MonoTypeOperatorFunction<T>;
Expand Down
11 changes: 6 additions & 5 deletions packages/core/rxjs-interop/src/rx_resource.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
BaseResourceOptions,
resource,
ResourceLoaderParams,
ResourceOptions,
ResourceRef,
ResourceStreamItem,
Signal,
Expand All @@ -26,9 +27,9 @@ import {encapsulateResourceError} from '../../src/resource/resource';
*
* @experimental
*/
export interface RxResourceOptions<T, R> extends BaseResourceOptions<T, R> {
export type RxResourceOptions<T, R> = BaseResourceOptions<T, R> & {
stream: (params: ResourceLoaderParams<R>) => Observable<T>;
}
};

/**
* Like `resource` but uses an RxJS based `loader` which maps the request to an `Observable` of the
Expand All @@ -38,7 +39,7 @@ export interface RxResourceOptions<T, R> extends BaseResourceOptions<T, R> {
*
* @experimental
*/
export function rxResource<T, R>(
export function rxResource<T, R = null>(
opts: RxResourceOptions<T, R> & {defaultValue: NoInfer<T>},
): ResourceRef<T>;

Expand All @@ -48,7 +49,7 @@ export function rxResource<T, R>(
*
* @experimental
*/
export function rxResource<T, R>(opts: RxResourceOptions<T, R>): ResourceRef<T | undefined>;
export function rxResource<T, R = null>(opts: RxResourceOptions<T, R>): ResourceRef<T | undefined>;
export function rxResource<T, R>(opts: RxResourceOptions<T, R>): ResourceRef<T | undefined> {
if (ngDevMode && !opts?.injector) {
assertInInjectionContext(rxResource);
Expand Down Expand Up @@ -108,5 +109,5 @@ export function rxResource<T, R>(opts: RxResourceOptions<T, R>): ResourceRef<T |

return promise;
},
});
} as ResourceOptions<T, R>);
}
55 changes: 55 additions & 0 deletions packages/core/rxjs-interop/test/rx_resource_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,61 @@ describe('rxResource()', () => {
});
});

describe('types', () => {
it('should type stream params as null when params option is omitted', () => {
rxResource({
stream: ({params}) => {
const _null: null = params;
return of('');
},
injector: TestBed.inject(Injector),
});
});

it('should type stream params correctly when params is provided', () => {
rxResource({
params: () => 'foo',
stream: ({params}) => {
const _str: string = params;
return of('');
},
injector: TestBed.inject(Injector),
});
});

it('should type stream params as null with explicit single generic', () => {
rxResource<string>({
stream: ({params}) => {
const _null: null = params;
return of('');
},
injector: TestBed.inject(Injector),
});
});
it('should infer only the param type as non-nullable', () => {
const condition = signal(true);
rxResource({
params: () => (condition() ? 'foo' : undefined),
stream: ({params}) => {
const _str: string = params;
return of('');
},
injector: TestBed.inject(Injector),
});
});

it('should error when two explicit generics are provided but params is absent', () => {
// @ts-expect-error: params is required when the second generic is not null
rxResource<string, string>({
stream: ({params}) => {
const _str: string = params;
return of('');
},
injector: TestBed.inject(Injector),
});
});
});

async function waitFor(fn: () => boolean): Promise<void> {
while (!fn()) {
await timeout(1);
Expand Down
29 changes: 15 additions & 14 deletions packages/core/src/resource/api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -207,15 +207,7 @@ export type ResourceStreamingLoader<T, R> = (
*
* @experimental
*/
export interface BaseResourceOptions<T, R> {
/**
* A reactive function which determines the request to be made. Whenever the request changes, the
* loader will be triggered to fetch a new value for the resource.
*
* If a params function isn't provided, the loader won't rerun unless the resource is reloaded.
*/
params?: (ctx: ResourceParamsContext) => R;

export type BaseResourceOptions<T, R> = {
/**
* The value which will be returned from the resource when a server value is unavailable, such as
* when the resource is still loading.
Expand All @@ -231,14 +223,14 @@ export interface BaseResourceOptions<T, R> {
* Overrides the `Injector` used by `resource`.
*/
injector?: Injector;
}
} & ResourceParams<R>;

/**
* Options to the `resource` function, for creating a resource.
*
* @experimental
*/
export interface PromiseResourceOptions<T, R> extends BaseResourceOptions<T, R> {
export type PromiseResourceOptions<T, R> = BaseResourceOptions<T, R> & {
/**
* Loading function which returns a `Promise` of the resource's value for a given request.
*/
Expand All @@ -248,14 +240,14 @@ export interface PromiseResourceOptions<T, R> extends BaseResourceOptions<T, R>
* Cannot specify `stream` and `loader` at the same time.
*/
stream?: never;
}
};

/**
* Options to the `resource` function, for creating a resource.
*
* @experimental
*/
export interface StreamingResourceOptions<T, R> extends BaseResourceOptions<T, R> {
export type StreamingResourceOptions<T, R> = BaseResourceOptions<T, R> & {
/**
* Loading function which returns a `Promise` of a signal of the resource's value for a given
* request, which can change over time as new values are received from a stream.
Expand All @@ -266,7 +258,16 @@ export interface StreamingResourceOptions<T, R> extends BaseResourceOptions<T, R
* Cannot specify `stream` and `loader` at the same time.
*/
loader?: never;
}
};

/**
*
*
* @experimental
*/
export type ResourceParams<R> = [R] extends [null]
? {params?: (ctx: ResourceParamsContext) => R}
: {params: (ctx: ResourceParamsContext) => R};

/**
* @experimental
Expand Down
14 changes: 8 additions & 6 deletions packages/core/src/resource/resource.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ import {linkedSignal} from '../render3/reactivity/linked_signal';
*
* @experimental 19.0
*/
export function resource<T, R>(
export function resource<T, R = null>(
options: ResourceOptions<T, R> & {defaultValue: NoInfer<T>},
): ResourceRef<T>;

Expand All @@ -61,8 +61,10 @@ export function resource<T, R>(
* @experimental 19.0
* @see [Async reactivity with resources](guide/signals/resource)
*/
Comment thread
JeanMeche marked this conversation as resolved.
export function resource<T, R>(options: ResourceOptions<T, R>): ResourceRef<T | undefined>;
export function resource<T, R>(options: ResourceOptions<T, R>): ResourceRef<T | undefined> {
export function resource<T, R = null>(options: ResourceOptions<T, R>): ResourceRef<T | undefined>;
export function resource<T, R>(
options: ResourceOptions<T, R> | ResourceOptions<T, never>,
): ResourceRef<T | undefined> {
if (ngDevMode && !options?.injector) {
assertInInjectionContext(resource);
}
Expand All @@ -73,7 +75,7 @@ export function resource<T, R>(options: ResourceOptions<T, R>): ResourceRef<T |
const params = options.params ?? oldNameForParams ?? (() => null!);
return new ResourceImpl<T | undefined, R>(
params,
getLoader(options),
getLoader(options as ResourceOptions<T, R>),
options.defaultValue,
options.equal ? wrapEqualityFn(options.equal) : undefined,
options.debugName,
Expand Down Expand Up @@ -506,7 +508,7 @@ function getLoader<T, R>(options: ResourceOptions<T, R>): ResourceStreamingLoade
return async (params) => {
try {
return signal(
{value: await options.loader(params)},
{value: await options!.loader(params)},
ngDevMode ? createDebugNameObject(options.debugName, 'stream') : undefined,
);
} catch (err) {
Expand All @@ -520,7 +522,7 @@ function getLoader<T, R>(options: ResourceOptions<T, R>): ResourceStreamingLoade

function isStreamingResourceOptions<T, R>(
options: ResourceOptions<T, R>,
): options is StreamingResourceOptions<T, R> {
): options is ResourceOptions<T, R> & StreamingResourceOptions<T, R> {
return !!(options as StreamingResourceOptions<T, R>).stream;
}

Expand Down
8 changes: 4 additions & 4 deletions packages/core/test/resource/params_status_spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ describe('resource with ResourceParamsStatus', () => {
const s = signal<string | ResourceParamsStatus>('foo');
const res = await act(() =>
resource({
params: throwStatusAndErrors(s),
params: throwStatusAndErrors<string>(s),
Copy link
Copy Markdown
Member

@JeanMeche JeanMeche Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've illustrated the different workarounds we have for this issue. (see the following changes too)

This comment was marked as off-topic.

Sign in to view

loader: async ({params}) => {
return params;
},
Expand All @@ -45,7 +45,7 @@ describe('resource with ResourceParamsStatus', () => {
let loadCount = 0;
const res = await act(() =>
resource({
params: throwStatusAndErrors(s),
params: throwStatusAndErrors(s)!,
loader: async ({params}) => {
loadCount++;
return params as string;
Expand All @@ -69,7 +69,7 @@ describe('resource with ResourceParamsStatus', () => {
const s = signal<string | Error>('foo');
const res = await act(() =>
resource({
params: throwStatusAndErrors(s),
params: throwStatusAndErrors(s) as () => string,
loader: async ({params}) => params as string,
injector: TestBed.inject(Injector),
}),
Expand All @@ -90,7 +90,7 @@ describe('resource with ResourceParamsStatus', () => {
let loadCount = 0;
const res = await act(() =>
resource({
params: throwStatusAndErrors(s),
params: throwStatusAndErrors(s)!,
loader: async ({params}) => {
loadCount++;
return params;
Expand Down
Loading
Loading