From 88685cb3b6d6fc09239ca9f7c1201fe89e79d2a8 Mon Sep 17 00:00:00 2001 From: Jessica Janiuk Date: Wed, 25 Feb 2026 14:00:05 -0800 Subject: [PATCH] fix(core): adds transfer cache to httpResource to fix hydration This should prevent the microtask problem with hydration and httpResource. fixes: #62897 --- packages/common/http/src/resource.ts | 43 ++++++++- packages/common/http/src/transfer_cache.ts | 96 ++++++++++++++----- packages/common/http/test/resource_spec.ts | 57 +++++++++++ packages/core/src/resource/resource.ts | 10 +- .../hydration/bundle.golden_symbols.json | 3 + 5 files changed, 179 insertions(+), 30 deletions(-) diff --git a/packages/common/http/src/resource.ts b/packages/common/http/src/resource.ts index 9b5704c932f..fb6dccf392c 100644 --- a/packages/common/http/src/resource.ts +++ b/packages/common/http/src/resource.ts @@ -20,6 +20,8 @@ import { ɵRuntimeError, ɵRuntimeErrorCode, ɵencapsulateResourceError as encapsulateResourceError, + TransferState, + untracked, } from '@angular/core'; import type {Subscription} from 'rxjs'; @@ -29,6 +31,11 @@ import {HttpErrorResponse, HttpEventType, HttpProgressEvent} from './response'; import {HttpHeaders} from './headers'; import {HttpParams} from './params'; import {HttpResourceRef, HttpResourceOptions, HttpResourceRequest} from './resource_api'; +import { + CACHE_OPTIONS, + HTTP_TRANSFER_CACHE_ORIGIN_MAP, + retrieveStateFromCache, +} from './transfer_cache'; /** * Type for the `httpRequest` top-level function, which includes the call signatures for the JSON- @@ -234,13 +241,41 @@ function makeHttpResourceFn(responseType: ResponseType) { assertInInjectionContext(httpResource); } const injector = options?.injector ?? inject(Injector); + + const cacheOptions = injector.get(CACHE_OPTIONS, null, {optional: true}); + const transferState = injector.get(TransferState, null, {optional: true}); + const originMap = injector.get(HTTP_TRANSFER_CACHE_ORIGIN_MAP, null, {optional: true}); + + const getInitialStream = (req: HttpRequest | undefined) => { + if (cacheOptions && transferState && req) { + const cachedResponse = retrieveStateFromCache(req, cacheOptions, transferState, originMap); + if (cachedResponse) { + try { + const body = cachedResponse.body as TRaw; + const parsed = options?.parse ? options.parse(body) : (body as unknown as TResult); + return signal({value: parsed}); + } catch (e) { + if (typeof ngDevMode === 'undefined' || ngDevMode) { + console.warn( + `Angular detected an error while parsing the cached response for the httpResource at \`${req.url}\`. ` + + `The resource will fall back to its default value and try again asynchronously.`, + e, + ); + } + } + } + } + return undefined; + }; + return new HttpResourceImpl( injector, () => normalizeRequest(request, responseType), - options?.defaultValue, + options?.defaultValue as TResult, options?.debugName, options?.parse as (value: unknown) => TResult, options?.equal as ValueEqualityFn, + getInitialStream, ) as HttpResourceRef; }; } @@ -321,11 +356,14 @@ class HttpResourceImpl constructor( injector: Injector, - request: () => HttpRequest | undefined, + request: () => HttpRequest | undefined, defaultValue: T, debugName?: string, parse?: (value: unknown) => T, equal?: ValueEqualityFn, + getInitialStream?: ( + request: HttpRequest | undefined, + ) => Signal> | undefined, ) { super( request, @@ -393,6 +431,7 @@ class HttpResourceImpl equal, debugName, injector, + getInitialStream, ); this.client = injector.get(HttpClient); } diff --git a/packages/common/http/src/transfer_cache.ts b/packages/common/http/src/transfer_cache.ts index 72c0ee1975b..575ce9ffe73 100644 --- a/packages/common/http/src/transfer_cache.ts +++ b/packages/common/http/src/transfer_cache.ts @@ -114,7 +114,7 @@ interface CacheOptions extends HttpTransferCacheOptions { isCacheActive: boolean; } -const CACHE_OPTIONS = new InjectionToken( +export const CACHE_OPTIONS = new InjectionToken( typeof ngDevMode !== 'undefined' && ngDevMode ? 'HTTP_TRANSFER_STATE_CACHE_OPTIONS' : '', ); @@ -123,14 +123,10 @@ const CACHE_OPTIONS = new InjectionToken( */ const ALLOWED_METHODS = ['GET', 'HEAD']; -export function transferCacheInterceptorFn( - req: HttpRequest, - next: HttpHandlerFn, -): Observable> { - const {isCacheActive, ...globalOptions} = inject(CACHE_OPTIONS); +function shouldCacheRequest(req: HttpRequest, options: CacheOptions): boolean { + const {isCacheActive, ...globalOptions} = options; const {transferCache: requestOptions, method: requestMethod} = req; - // In the following situations we do not want to cache the request if ( !isCacheActive || requestOptions === false || @@ -141,14 +137,37 @@ export function transferCacheInterceptorFn( (!globalOptions.includeRequestsWithAuthHeaders && hasAuthHeaders(req)) || globalOptions.filter?.(req) === false ) { - return next(req); + return false; } - const transferState = inject(TransferState); + return true; +} - const originMap: Record | null = inject(HTTP_TRANSFER_CACHE_ORIGIN_MAP, { - optional: true, - }); +function getHeadersToInclude( + options: CacheOptions, + requestOptions: HttpTransferCacheOptions | boolean | undefined, +): string[] | undefined { + const {includeHeaders: globalHeaders} = options; + let headersToInclude = globalHeaders; + if (typeof requestOptions === 'object' && requestOptions.includeHeaders) { + // Request-specific config takes precedence over the global config. + headersToInclude = requestOptions.includeHeaders; + } + return headersToInclude; +} + +export function retrieveStateFromCache( + req: HttpRequest, + options: CacheOptions, + transferState: TransferState, + originMap: Record | null, +): HttpResponse | null { + const {transferCache: requestOptions} = req; + + // In the following situations we do not want to cache the request + if (!shouldCacheRequest(req, options)) { + return null; + } if (typeof ngServerMode !== 'undefined' && !ngServerMode && originMap) { throw new RuntimeError( @@ -168,11 +187,7 @@ export function transferCacheInterceptorFn( const storeKey = makeCacheKey(req, requestUrl); const response = transferState.get(storeKey, null); - let headersToInclude = globalOptions.includeHeaders; - if (typeof requestOptions === 'object' && requestOptions.includeHeaders) { - // Request-specific config takes precedence over the global config. - headersToInclude = requestOptions.includeHeaders; - } + const headersToInclude = getHeadersToInclude(options, requestOptions); if (response) { const { @@ -206,15 +221,44 @@ export function transferCacheInterceptorFn( headers = appendMissingHeadersDetection(req.url, headers, headersToInclude ?? []); } - return of( - new HttpResponse({ - body, - headers, - status, - statusText, - url, - }), - ); + return new HttpResponse({ + body, + headers, + status, + statusText, + url, + }); + } + + return null; +} + +export function transferCacheInterceptorFn( + req: HttpRequest, + next: HttpHandlerFn, +): Observable> { + const options = inject(CACHE_OPTIONS); + const transferState = inject(TransferState); + + const originMap = inject(HTTP_TRANSFER_CACHE_ORIGIN_MAP, {optional: true}); + + const cachedResponse = retrieveStateFromCache(req, options, transferState, originMap); + if (cachedResponse) { + return of(cachedResponse); + } + + const {transferCache: requestOptions} = req; + const headersToInclude = getHeadersToInclude(options, requestOptions); + + const requestUrl = + typeof ngServerMode !== 'undefined' && ngServerMode && originMap + ? mapRequestOriginUrl(req.url, originMap) + : req.url; + const storeKey = makeCacheKey(req, requestUrl); + + // In the following situations we do not want to cache the request + if (!shouldCacheRequest(req, options)) { + return next(req); } const event$ = next(req); diff --git a/packages/common/http/test/resource_spec.ts b/packages/common/http/test/resource_spec.ts index 8ed1f8a0884..5e0514bddd7 100644 --- a/packages/common/http/test/resource_spec.ts +++ b/packages/common/http/test/resource_spec.ts @@ -18,6 +18,8 @@ import { HttpResourceRef, } from '../index'; import {HttpTestingController, provideHttpClientTesting} from '../testing'; +import {withHttpTransferCache} from '../src/transfer_cache'; +import {HttpClient} from '../src/client'; describe('httpResource', () => { beforeEach(() => { @@ -400,4 +402,59 @@ describe('httpResource', () => { } }); }); + + describe('TransferCache integration', () => { + beforeEach(() => { + TestBed.resetTestingModule(); + TestBed.configureTestingModule({ + providers: [provideHttpClient(), provideHttpClientTesting(), withHttpTransferCache({})], + }); + }); + + it('should synchronously resolve with a cached value from TransferState', async () => { + globalThis['ngServerMode'] = true; + let requestResolved = false; + TestBed.inject(HttpClient) + .get('/data') + .subscribe(() => (requestResolved = true)); + const req = TestBed.inject(HttpTestingController).expectOne('/data'); + req.flush([1, 2, 3]); + + expect(requestResolved).toBe(true); + + // Now switch to client mode + globalThis['ngServerMode'] = false; + + // Create httpResource. It should immediately read from TransferState. + const res = httpResource(() => '/data', {injector: TestBed.inject(Injector)}); + + // It should immediately have the value synchronously and status should be resolved + expect(res.status()).toBe('resolved'); + expect(res.hasValue()).toBe(true); + expect(res.value()).toEqual([1, 2, 3]); + + // Also no new request should be made + TestBed.inject(HttpTestingController).expectNone('/data'); + }); + + it('should not evaluate the request payload during resource initialization', () => { + let requestEvaluated = false; + const res = httpResource( + () => { + requestEvaluated = true; + return '/data'; + }, + {injector: TestBed.inject(Injector)}, + ); + + // Request function should NOT be evaluated during initialization + expect(requestEvaluated).toBe(false); + + // Read to trigger it + res.status(); + + // The request should now have been evaluated + expect(requestEvaluated).toBe(true); + }); + }); }); diff --git a/packages/core/src/resource/resource.ts b/packages/core/src/resource/resource.ts index bbe66d126e6..4448130438b 100644 --- a/packages/core/src/resource/resource.ts +++ b/packages/core/src/resource/resource.ts @@ -196,6 +196,7 @@ export class ResourceImpl extends BaseWritableResource implements Resou private readonly equal: ValueEqualityFn | undefined, private readonly debugName: string | undefined, injector: Injector, + getInitialStream?: (request: R) => Signal> | undefined, ) { super( // Feed a computed signal for the value to `BaseWritableResource`, which will upgrade it to a @@ -238,15 +239,20 @@ 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 ? 'idle' : 'loading'; if (!previous) { + const initialStream = getInitialStream?.(extRequest.request as R); + // Clear getInitialStream so it doesn't hold onto memory + getInitialStream = undefined; + const status = + extRequest.request === undefined ? 'idle' : initialStream ? 'resolved' : 'loading'; return { extRequest, status, previousStatus: 'idle', - stream: undefined, + stream: initialStream, }; } else { + const status = extRequest.request === undefined ? 'idle' : 'loading'; return { extRequest, status, diff --git a/packages/core/test/bundling/hydration/bundle.golden_symbols.json b/packages/core/test/bundling/hydration/bundle.golden_symbols.json index 112631ad478..20e1a0d8331 100644 --- a/packages/core/test/bundling/hydration/bundle.golden_symbols.json +++ b/packages/core/test/bundling/hydration/bundle.golden_symbols.json @@ -476,6 +476,7 @@ "getFactoryDef", "getFirstLContainer", "getGlobalLocale", + "getHeadersToInclude", "getInheritedInjectableDef", "getInitialLViewFlagsFromDef", "getInjectFlag", @@ -722,6 +723,7 @@ "resolveForwardRef", "retrieveHydrationInfo", "retrieveHydrationInfoImpl", + "retrieveStateFromCache", "retrieveTransferredState", "runAfterLeaveAnimations", "runEffectsInView", @@ -770,6 +772,7 @@ "shimHostAttribute", "shimStylesContent", "shouldBeIgnoredByZone", + "shouldCacheRequest", "shouldSearchParent", "siblingAfter", "skipTextNodes",