From d8ca560a15767b22dcf2e7aa0b20092c347176ef Mon Sep 17 00:00:00 2001 From: Alex Rickabaugh Date: Fri, 18 Apr 2025 14:15:22 -0700 Subject: [PATCH] refactor(core): convert `ResourceStatus` to a string type (#60919) An outcome of the Resource RFC was that we should use string constants for communicating the resource status instead of an enum. This commit converts `ResourceStatus` accordingly. PR Close #60919 --- adev/src/content/guide/signals/resource.md | 21 +++-- goldens/public-api/core/index.api.md | 9 +- packages/common/http/src/resource.ts | 4 +- packages/core/src/resource/api.ts | 60 +++++-------- packages/core/src/resource/resource.ts | 48 +++++------ packages/core/test/resource/resource_spec.ts | 88 ++++++++++---------- 6 files changed, 95 insertions(+), 135 deletions(-) diff --git a/adev/src/content/guide/signals/resource.md b/adev/src/content/guide/signals/resource.md index e06c100c9a7..d0b70ac6d0f 100644 --- a/adev/src/content/guide/signals/resource.md +++ b/adev/src/content/guide/signals/resource.md @@ -47,8 +47,7 @@ The `ResourceLoaderParams` object contains three properties: `request`, `previou | `previous` | An object with a `status` property, containing the previous `ResourceStatus`. | | `abortSignal` | An [`AbortSignal`](https://developer.mozilla.org/en-US/docs/Web/API/AbortSignal). See [Aborting requests](#aborting-requests) below for details. | - -If the `request` computation returns `undefined`, the loader function does not run and the resource status becomes `Idle`. +If the `request` computation returns `undefined`, the loader function does not run and the resource status becomes `'idle'`. ### Aborting requests @@ -100,15 +99,15 @@ The resource object has several signal properties for reading the status of the | `isLoading` | Whether the resource loader is currently running. | | `status` | The resource's specific `ResourceStatus`, as described below. | -The `status` signal provides a specific `ResourceStatus` that describes the state of the resource. +The `status` signal provides a specific `ResourceStatus` that describes the state of the resource using a string constant. -| Status | `value()` | Description | -| ----------- | :---------------- | ---------------------------------------------------------------------------- | -| `Idle` | `undefined` | The resource has no valid request and the loader has not run. | -| `Error` | `undefined` | The loader has encountered an error. | -| `Loading` | `undefined` | The loader is running as a result of the `request` value changing. | -| `Reloading` | Previous value | The loader is running as a result calling of the resource's `reload` method. | -| `Resolved` | Resolved value | The loader has completed. | -| `Local` | Locally set value | The resource's value has been set locally via `.set()` or `.update()` | +| Status | `value()` | Description | +| ------------- | :---------------- | ---------------------------------------------------------------------------- | +| `'idle'` | `undefined` | The resource has no valid request and the loader has not run. | +| `'error'` | `undefined` | The loader has encountered an error. | +| `'loading'` | `undefined` | The loader is running as a result of the `request` value changing. | +| `'reloading'` | Previous value | The loader is running as a result calling of the resource's `reload` method. | +| `'resolved'` | Resolved value | The loader has completed. | +| `'local'` | Locally set value | The resource's value has been set locally via `.set()` or `.update()` | You can use this status information to conditionally display user interface elements, such loading indicators and error messages. diff --git a/goldens/public-api/core/index.api.md b/goldens/public-api/core/index.api.md index 66813f21f4c..86e7c4f89a2 100644 --- a/goldens/public-api/core/index.api.md +++ b/goldens/public-api/core/index.api.md @@ -1638,14 +1638,7 @@ export interface ResourceRef extends WritableResource { } // @public -export enum ResourceStatus { - Error = 1, - Idle = 0, - Loading = 2, - Local = 5, - Reloading = 3, - Resolved = 4 -} +export type ResourceStatus = 'idle' | 'error' | 'loading' | 'reloading' | 'resolved' | 'local'; // @public export type ResourceStreamingLoader = (param: ResourceLoaderParams) => PromiseLike>>; diff --git a/packages/common/http/src/resource.ts b/packages/common/http/src/resource.ts index ca3544aca42..7b10d6bb35d 100644 --- a/packages/common/http/src/resource.ts +++ b/packages/common/http/src/resource.ts @@ -301,9 +301,7 @@ class HttpResourceImpl }); readonly headers = computed(() => - this.status() === ResourceStatus.Resolved || this.status() === ResourceStatus.Error - ? this._headers() - : undefined, + this.status() === 'resolved' || this.status() === 'error' ? this._headers() : undefined, ); readonly progress = this._progress.asReadonly(); readonly statusCode = this._statusCode.asReadonly(); diff --git a/packages/core/src/resource/api.ts b/packages/core/src/resource/api.ts index a6cbab70cb6..ebe1f7107be 100644 --- a/packages/core/src/resource/api.ts +++ b/packages/core/src/resource/api.ts @@ -11,49 +11,29 @@ import {Signal, ValueEqualityFn} from '../render3/reactivity/api'; import {WritableSignal} from '../render3/reactivity/signal'; /** - * Status of a `Resource`. + * String value capturing the status of a `Resource`. + * + * Possible statuses are: + * + * `idle` - The resource has no valid request and will not perform any loading. `value()` will be + * `undefined`. + * + * `loading` - The resource is currently loading a new value as a result of a change in its reactive + * dependencies. `value()` will be `undefined`. + * + * `reloading` - The resource is currently reloading a fresh value for the same reactive + * dependencies. `value()` will continue to return the previously fetched value during the reloading + * operation. + * + * `error` - Loading failed with an error. `value()` will be `undefined`. + * + * `resolved` - Loading has completed and the resource has the value returned from the loader. + * + * `local` - The resource's value was set locally via `.set()` or `.update()`. * * @experimental */ -export enum ResourceStatus { - /** - * The resource has no valid request and will not perform any loading. - * - * `value()` will be `undefined`. - */ - Idle, - - /** - * Loading failed with an error. - * - * `value()` will be `undefined`. - */ - Error, - - /** - * The resource is currently loading a new value as a result of a change in its `request`. - * - * `value()` will be `undefined`. - */ - Loading, - - /** - * The resource is currently reloading a fresh value for the same request. - * - * `value()` will continue to return the previously fetched value during the reloading operation. - */ - Reloading, - - /** - * Loading has completed and the resource has the value returned from the loader. - */ - Resolved, - - /** - * The resource's value was set locally via `.set()` or `.update()`. - */ - Local, -} +export type ResourceStatus = 'idle' | 'error' | 'loading' | 'reloading' | 'resolved' | 'local'; /** * A Resource is an asynchronous dependency (for example, the results of an API call) that is diff --git a/packages/core/src/resource/resource.ts b/packages/core/src/resource/resource.ts index 098fcce24c7..b269e9dd55f 100644 --- a/packages/core/src/resource/resource.ts +++ b/packages/core/src/resource/resource.ts @@ -68,11 +68,7 @@ export function resource(options: ResourceOptions): ResourceRef implements WritableResource { this.set(updateFn(untracked(this.value))); } - readonly isLoading = computed( - () => this.status() === ResourceStatus.Loading || this.status() === ResourceStatus.Reloading, - ); + readonly isLoading = computed(() => this.status() === 'loading' || this.status() === 'reloading'); hasValue(): this is ResourceRef> { return this.value() !== undefined; @@ -181,13 +175,12 @@ export class ResourceImpl extends BaseWritableResource implements Resou source: this.extRequest, // Compute the state of the resource given a change in status. computation: (extRequest, previous) => { - const status = - extRequest.request === undefined ? ResourceStatus.Idle : ResourceStatus.Loading; + const status = extRequest.request === undefined ? 'idle' : 'loading'; if (!previous) { return { extRequest, status, - previousStatus: ResourceStatus.Idle, + previousStatus: 'idle', stream: undefined, }; } else { @@ -234,18 +227,15 @@ export class ResourceImpl extends BaseWritableResource implements Resou const current = untracked(this.value); const state = untracked(this.state); - if ( - state.status === ResourceStatus.Local && - (this.equal ? this.equal(current, value) : current === value) - ) { + if (state.status === 'local' && (this.equal ? this.equal(current, value) : current === value)) { return; } // Enter Local state with the user-defined value. this.state.set({ extRequest: state.extRequest, - status: ResourceStatus.Local, - previousStatus: ResourceStatus.Local, + status: 'local', + previousStatus: 'local', stream: signal({value}), }); @@ -257,7 +247,7 @@ export class ResourceImpl extends BaseWritableResource implements Resou override reload(): boolean { // We don't want to restart in-progress loads. const {status} = untracked(this.state); - if (status === ResourceStatus.Idle || status === ResourceStatus.Loading) { + if (status === 'idle' || status === 'loading') { return false; } @@ -274,8 +264,8 @@ export class ResourceImpl extends BaseWritableResource implements Resou // Destroyed resources enter Idle state. this.state.set({ extRequest: {request: undefined, reload: 0}, - status: ResourceStatus.Idle, - previousStatus: ResourceStatus.Idle, + status: 'idle', + previousStatus: 'idle', stream: undefined, }); } @@ -290,7 +280,7 @@ export class ResourceImpl extends BaseWritableResource implements Resou if (extRequest.request === undefined) { // Nothing to load (and we should already be in a non-loading state). return; - } else if (currentStatus !== ResourceStatus.Loading) { + } else if (currentStatus !== 'loading') { // We're not in a loading or reloading state, so this loading request is stale. return; } @@ -334,8 +324,8 @@ export class ResourceImpl extends BaseWritableResource implements Resou this.state.set({ extRequest, - status: ResourceStatus.Resolved, - previousStatus: ResourceStatus.Resolved, + status: 'resolved', + previousStatus: 'resolved', stream, }); } catch (err) { @@ -345,8 +335,8 @@ export class ResourceImpl extends BaseWritableResource implements Resou this.state.set({ extRequest, - status: ResourceStatus.Resolved, - previousStatus: ResourceStatus.Error, + status: 'resolved', + previousStatus: 'error', stream: signal({error: err}), }); } finally { @@ -398,10 +388,10 @@ function isStreamingResourceOptions( */ function projectStatusOfState(state: ResourceState): ResourceStatus { switch (state.status) { - case ResourceStatus.Loading: - return state.extRequest.reload === 0 ? ResourceStatus.Loading : ResourceStatus.Reloading; - case ResourceStatus.Resolved: - return isResolved(untracked(state.stream!)) ? ResourceStatus.Resolved : ResourceStatus.Error; + case 'loading': + return state.extRequest.reload === 0 ? 'loading' : 'reloading'; + case 'resolved': + return isResolved(untracked(state.stream!)) ? 'resolved' : 'error'; default: return state.status; } diff --git a/packages/core/test/resource/resource_spec.ts b/packages/core/test/resource/resource_spec.ts index 177b1ee1688..ee90d9ff687 100644 --- a/packages/core/test/resource/resource_spec.ts +++ b/packages/core/test/resource/resource_spec.ts @@ -84,14 +84,14 @@ describe('resource', () => { }); // a freshly created resource is in the loading state - expect(echoResource.status()).toBe(ResourceStatus.Loading); + expect(echoResource.status()).toBe('loading'); expect(echoResource.isLoading()).toBeTrue(); expect(echoResource.hasValue()).toBeFalse(); expect(echoResource.value()).toBeUndefined(); expect(echoResource.error()).toBe(undefined); TestBed.tick(); await backend.flush(); - expect(echoResource.status()).toBe(ResourceStatus.Resolved); + expect(echoResource.status()).toBe('resolved'); expect(echoResource.isLoading()).toBeFalse(); expect(echoResource.hasValue()).toBeTrue(); expect(echoResource.value()).toEqual({counter: 0}); @@ -100,7 +100,7 @@ describe('resource', () => { counter.update((c) => c + 1); TestBed.tick(); await backend.flush(); - expect(echoResource.status()).toBe(ResourceStatus.Resolved); + expect(echoResource.status()).toBe('resolved'); expect(echoResource.isLoading()).toBeFalse(); expect(echoResource.hasValue()).toBeTrue(); expect(echoResource.value()).toEqual({counter: 1}); @@ -123,7 +123,7 @@ describe('resource', () => { TestBed.tick(); await flushMicrotasks(); - expect(prevStatus).toBe(ResourceStatus.Idle); + expect(prevStatus).toBe('idle'); }); it('should expose errors thrown during resource loading', async () => { @@ -138,7 +138,7 @@ describe('resource', () => { TestBed.tick(); await backend.reject(requestParam, 'Something went wrong....'); - expect(echoResource.status()).toBe(ResourceStatus.Error); + expect(echoResource.status()).toBe('error'); expect(echoResource.isLoading()).toBeFalse(); expect(echoResource.hasValue()).toBeFalse(); expect(echoResource.value()).toEqual(undefined); @@ -163,7 +163,7 @@ describe('resource', () => { TestBed.tick(); await backend.flush(); - expect(echoResource.status()).toBe(ResourceStatus.Resolved); + expect(echoResource.status()).toBe('resolved'); expect(echoResource.isLoading()).toBeFalse(); expect(echoResource.hasValue()).toBeTrue(); expect(echoResource.value()).toEqual('ok'); @@ -173,7 +173,7 @@ describe('resource', () => { TestBed.tick(); await backend.flush(); - expect(echoResource.status()).toBe(ResourceStatus.Error); + expect(echoResource.status()).toBe('error'); expect(echoResource.isLoading()).toBeFalse(); expect(echoResource.hasValue()).toBeFalse(); expect(echoResource.value()).toEqual(undefined); @@ -210,9 +210,9 @@ describe('resource', () => { // The resource should still be loading. Ticking (triggering the 2nd effect) // should not change the loading status. - expect(res.status()).toBe(ResourceStatus.Loading); + expect(res.status()).toBe('loading'); appRef.tick(); - expect(res.status()).toBe(ResourceStatus.Loading); + expect(res.status()).toBe('loading'); expect(resolve.length).toBe(2); // Resolve the second load. @@ -220,7 +220,7 @@ describe('resource', () => { await flushMicrotasks(); // We should see the resolved value. - expect(res.status()).toBe(ResourceStatus.Resolved); + expect(res.status()).toBe('resolved'); expect(res.value()).toBe(1); }); @@ -262,7 +262,7 @@ describe('resource', () => { }); TestBed.tick(); - expect(echoResource.status()).toBe(ResourceStatus.Idle); + expect(echoResource.status()).toBe('idle'); expect(echoResource.isLoading()).toBeFalse(); counter.set(10); @@ -296,7 +296,7 @@ describe('resource', () => { counter.update((c) => c + 1); TestBed.tick(); await backend.flush(); - expect(echoResource.status()).toBe(ResourceStatus.Resolved); + expect(echoResource.status()).toBe('resolved'); expect(echoResource.value()).toEqual({counter: 1}); expect(echoResource.error()).toBe(undefined); @@ -328,7 +328,7 @@ describe('resource', () => { injector.destroy(); await backend.flush(); - expect(echoResource.status()).toBe(ResourceStatus.Idle); + expect(echoResource.status()).toBe('idle'); expect(echoResource.value()).toBe(undefined); expect(echoResource.error()).toBe(undefined); @@ -361,7 +361,7 @@ describe('resource', () => { echoResource.destroy(); await backend.flush(); - expect(echoResource.status()).toBe(ResourceStatus.Idle); + expect(echoResource.status()).toBe('idle'); expect(echoResource.value()).toBe(undefined); expect(echoResource.error()).toBe(undefined); @@ -388,7 +388,7 @@ describe('resource', () => { unrelated.set('b'); TestBed.tick(); // there is no chang in the status - expect(res.status()).toBe(ResourceStatus.Resolved); + expect(res.status()).toBe('resolved'); await backend.flush(); // there is no chang in the value expect(res.value()).toBe('0:0'); @@ -406,13 +406,13 @@ describe('resource', () => { TestBed.tick(); await backend.flush(); - expect(echoResource.status()).toBe(ResourceStatus.Resolved); + expect(echoResource.status()).toBe('resolved'); expect(echoResource.isLoading()).toBeFalse(); expect(echoResource.value()).toEqual({counter: 0}); expect(echoResource.error()).toBe(undefined); echoResource.value.set({counter: 100}); - expect(echoResource.status()).toBe(ResourceStatus.Local); + expect(echoResource.status()).toBe('local'); expect(echoResource.isLoading()).toBeFalse(); expect(echoResource.hasValue()).toBeTrue(); expect(echoResource.value()).toEqual({counter: 100}); @@ -421,13 +421,13 @@ describe('resource', () => { counter.set(1); TestBed.tick(); await backend.flush(); - expect(echoResource.status()).toBe(ResourceStatus.Resolved); + expect(echoResource.status()).toBe('resolved'); expect(echoResource.value()).toEqual({counter: 1}); expect(echoResource.error()).toBe(undefined); // state setter is also exposed on the resource directly echoResource.set({counter: 200}); - expect(echoResource.status()).toBe(ResourceStatus.Local); + expect(echoResource.status()).toBe('local'); expect(echoResource.hasValue()).toBeTrue(); expect(echoResource.value()).toEqual({counter: 200}); }); @@ -442,17 +442,17 @@ describe('resource', () => { TestBed.tick(); await backend.flush(); - expect(res.status()).toBe(ResourceStatus.Resolved); + expect(res.status()).toBe('resolved'); expect(res.value()).toBe('0:0'); expect(res.error()).toBe(undefined); res.reload(); - expect(res.status()).toBe(ResourceStatus.Reloading); + expect(res.status()).toBe('reloading'); expect(res.value()).toBe('0:0'); TestBed.tick(); await backend.flush(); - expect(res.status()).toBe(ResourceStatus.Resolved); + expect(res.status()).toBe('resolved'); expect(res.isLoading()).toBeFalse(); expect(res.value()).toBe('0:1'); expect(res.error()).toBe(undefined); @@ -474,12 +474,12 @@ describe('resource', () => { }); res.value.set(5); - expect(res.status()).toBe(ResourceStatus.Local); + expect(res.status()).toBe('local'); expect(res.value()).toBe(5); expect(res.error()).toBe(undefined); res.value.set(10); - expect(res.status()).toBe(ResourceStatus.Local); + expect(res.status()).toBe('local'); expect(res.value()).toBe(5); // equality blocked writes expect(res.error()).toBe(undefined); }); @@ -509,24 +509,24 @@ describe('resource', () => { injector: TestBed.inject(Injector), }); // Idle to start. - expect(echoResource.status()).toBe(ResourceStatus.Idle); + expect(echoResource.status()).toBe('idle'); // Switch to loading state should be synchronous. request.set(1); - expect(echoResource.status()).toBe(ResourceStatus.Loading); + expect(echoResource.status()).toBe('loading'); // And back to idle. request.set(undefined); - expect(echoResource.status()).toBe(ResourceStatus.Idle); + expect(echoResource.status()).toBe('idle'); // Allow the load to proceed. request.set(2); TestBed.tick(); await backend.flush(); - expect(echoResource.status()).toBe(ResourceStatus.Resolved); + expect(echoResource.status()).toBe('resolved'); // Reload state should be synchronous. echoResource.reload(); - expect(echoResource.status()).toBe(ResourceStatus.Reloading); + expect(echoResource.status()).toBe('reloading'); // Back to idle. request.set(undefined); - expect(echoResource.status()).toBe(ResourceStatus.Idle); + expect(echoResource.status()).toBe('idle'); }); it('set() should abort a pending load', async () => { const request = signal(1); @@ -540,22 +540,22 @@ describe('resource', () => { // Fully resolve the resource to start. TestBed.tick(); await backend.flush(); - expect(echoResource.status()).toBe(ResourceStatus.Resolved); + expect(echoResource.status()).toBe('resolved'); // Trigger loading state. request.set(2); - expect(echoResource.status()).toBe(ResourceStatus.Loading); + expect(echoResource.status()).toBe('loading'); // Set the resource to a new value. echoResource.set(3); // Now run the effect, which should be a no-op as the resource was set to a local value. TestBed.tick(); // We should still be in local state. - expect(echoResource.status()).toBe(ResourceStatus.Local); + expect(echoResource.status()).toBe('local'); expect(echoResource.value()).toBe(3); // Flush the resource await backend.flush(); await appRef.whenStable(); // We should still be in local state. - expect(echoResource.status()).toBe(ResourceStatus.Local); + expect(echoResource.status()).toBe('local'); expect(echoResource.value()).toBe(3); }); @@ -571,22 +571,22 @@ describe('resource', () => { // Fully resolve the resource to start. TestBed.tick(); await backend.flush(); - expect(echoResource.status()).toBe(ResourceStatus.Resolved); + expect(echoResource.status()).toBe('resolved'); // Trigger reloading state. echoResource.reload(); - expect(echoResource.status()).toBe(ResourceStatus.Reloading); + expect(echoResource.status()).toBe('reloading'); // Set the resource to a new value. echoResource.set(3); // Now run the effect, which should be a no-op as the resource was set to a local value. TestBed.tick(); // We should still be in local state. - expect(echoResource.status()).toBe(ResourceStatus.Local); + expect(echoResource.status()).toBe('local'); expect(echoResource.value()).toBe(3); // Flush the resource await backend.flush(); await appRef.whenStable(); // We should still be in local state. - expect(echoResource.status()).toBe(ResourceStatus.Local); + expect(echoResource.status()).toBe('local'); expect(echoResource.value()).toBe(3); }); @@ -598,7 +598,7 @@ describe('resource', () => { }); await appRef.whenStable(); - expect(res.status()).toBe(ResourceStatus.Resolved); + expect(res.status()).toBe('resolved'); expect(res.value()).toBe('done'); }); @@ -610,7 +610,7 @@ describe('resource', () => { }); await appRef.whenStable(); - expect(res.status()).toBe(ResourceStatus.Error); + expect(res.status()).toBe('error'); expect(res.error()).toBe('fail'); }); @@ -688,7 +688,7 @@ describe('resource', () => { // Start the initial load. TestBed.tick(); await Promise.resolve(); - expect(echoResource.status()).toBe(ResourceStatus.Loading); + expect(echoResource.status()).toBe('loading'); expect(echoResource.value()).toBe(undefined); expect(echoResource.error()).toBe(undefined); expect(aborted).toEqual([]); @@ -697,7 +697,7 @@ describe('resource', () => { echoResource.set(null); TestBed.tick(); await backend.flush(); - expect(echoResource.status()).toBe(ResourceStatus.Local); + expect(echoResource.status()).toBe('local'); expect(echoResource.value()).toBe(null); expect(echoResource.error()).toBe(undefined); expect(aborted).toEqual([{counter: 0}]); @@ -706,7 +706,7 @@ describe('resource', () => { echoResource.reload(); TestBed.tick(); await Promise.resolve(); - expect(echoResource.status()).toBe(ResourceStatus.Reloading); + expect(echoResource.status()).toBe('reloading'); expect(echoResource.value()).toBe(null); expect(echoResource.error()).toBe(undefined); expect(aborted).toEqual([{counter: 0}]); @@ -714,7 +714,7 @@ describe('resource', () => { // Interrupt the reload with the same value as before. echoResource.set(null); await backend.flush(); - expect(echoResource.status()).toBe(ResourceStatus.Local); + expect(echoResource.status()).toBe('local'); expect(echoResource.value()).toBe(null); expect(echoResource.error()).toBe(undefined); expect(aborted).toEqual([{counter: 0}, {counter: 0}]);