Skip to content

Commit 88685cb

Browse files
thePunderWomanthePunderWoman
committed
fix(core): adds transfer cache to httpResource to fix hydration
This should prevent the microtask problem with hydration and httpResource. fixes: #62897
1 parent e1bc342 commit 88685cb

File tree

5 files changed

+179
-30
lines changed

5 files changed

+179
-30
lines changed

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

Lines changed: 41 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,8 @@ import {
2020
ɵRuntimeError,
2121
ɵRuntimeErrorCode,
2222
ɵencapsulateResourceError as encapsulateResourceError,
23+
TransferState,
24+
untracked,
2325
} from '@angular/core';
2426
import type {Subscription} from 'rxjs';
2527

@@ -29,6 +31,11 @@ import {HttpErrorResponse, HttpEventType, HttpProgressEvent} from './response';
2931
import {HttpHeaders} from './headers';
3032
import {HttpParams} from './params';
3133
import {HttpResourceRef, HttpResourceOptions, HttpResourceRequest} from './resource_api';
34+
import {
35+
CACHE_OPTIONS,
36+
HTTP_TRANSFER_CACHE_ORIGIN_MAP,
37+
retrieveStateFromCache,
38+
} from './transfer_cache';
3239

3340
/**
3441
* Type for the `httpRequest` top-level function, which includes the call signatures for the JSON-
@@ -234,13 +241,41 @@ function makeHttpResourceFn<TRaw>(responseType: ResponseType) {
234241
assertInInjectionContext(httpResource);
235242
}
236243
const injector = options?.injector ?? inject(Injector);
244+
245+
const cacheOptions = injector.get(CACHE_OPTIONS, null, {optional: true});
246+
const transferState = injector.get(TransferState, null, {optional: true});
247+
const originMap = injector.get(HTTP_TRANSFER_CACHE_ORIGIN_MAP, null, {optional: true});
248+
249+
const getInitialStream = (req: HttpRequest<unknown> | undefined) => {
250+
if (cacheOptions && transferState && req) {
251+
const cachedResponse = retrieveStateFromCache(req, cacheOptions, transferState, originMap);
252+
if (cachedResponse) {
253+
try {
254+
const body = cachedResponse.body as TRaw;
255+
const parsed = options?.parse ? options.parse(body) : (body as unknown as TResult);
256+
return signal({value: parsed});
257+
} catch (e) {
258+
if (typeof ngDevMode === 'undefined' || ngDevMode) {
259+
console.warn(
260+
`Angular detected an error while parsing the cached response for the httpResource at \`${req.url}\`. ` +
261+
`The resource will fall back to its default value and try again asynchronously.`,
262+
e,
263+
);
264+
}
265+
}
266+
}
267+
}
268+
return undefined;
269+
};
270+
237271
return new HttpResourceImpl(
238272
injector,
239273
() => normalizeRequest(request, responseType),
240-
options?.defaultValue,
274+
options?.defaultValue as TResult,
241275
options?.debugName,
242276
options?.parse as (value: unknown) => TResult,
243277
options?.equal as ValueEqualityFn<unknown>,
278+
getInitialStream,
244279
) as HttpResourceRef<TResult>;
245280
};
246281
}
@@ -321,11 +356,14 @@ class HttpResourceImpl<T>
321356

322357
constructor(
323358
injector: Injector,
324-
request: () => HttpRequest<T> | undefined,
359+
request: () => HttpRequest<unknown> | undefined,
325360
defaultValue: T,
326361
debugName?: string,
327362
parse?: (value: unknown) => T,
328363
equal?: ValueEqualityFn<unknown>,
364+
getInitialStream?: (
365+
request: HttpRequest<unknown> | undefined,
366+
) => Signal<ResourceStreamItem<T>> | undefined,
329367
) {
330368
super(
331369
request,
@@ -393,6 +431,7 @@ class HttpResourceImpl<T>
393431
equal,
394432
debugName,
395433
injector,
434+
getInitialStream,
396435
);
397436
this.client = injector.get(HttpClient);
398437
}

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

Lines changed: 70 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -114,7 +114,7 @@ interface CacheOptions extends HttpTransferCacheOptions {
114114
isCacheActive: boolean;
115115
}
116116

117-
const CACHE_OPTIONS = new InjectionToken<CacheOptions>(
117+
export const CACHE_OPTIONS = new InjectionToken<CacheOptions>(
118118
typeof ngDevMode !== 'undefined' && ngDevMode ? 'HTTP_TRANSFER_STATE_CACHE_OPTIONS' : '',
119119
);
120120

@@ -123,14 +123,10 @@ const CACHE_OPTIONS = new InjectionToken<CacheOptions>(
123123
*/
124124
const ALLOWED_METHODS = ['GET', 'HEAD'];
125125

126-
export function transferCacheInterceptorFn(
127-
req: HttpRequest<unknown>,
128-
next: HttpHandlerFn,
129-
): Observable<HttpEvent<unknown>> {
130-
const {isCacheActive, ...globalOptions} = inject(CACHE_OPTIONS);
126+
function shouldCacheRequest(req: HttpRequest<unknown>, options: CacheOptions): boolean {
127+
const {isCacheActive, ...globalOptions} = options;
131128
const {transferCache: requestOptions, method: requestMethod} = req;
132129

133-
// In the following situations we do not want to cache the request
134130
if (
135131
!isCacheActive ||
136132
requestOptions === false ||
@@ -141,14 +137,37 @@ export function transferCacheInterceptorFn(
141137
(!globalOptions.includeRequestsWithAuthHeaders && hasAuthHeaders(req)) ||
142138
globalOptions.filter?.(req) === false
143139
) {
144-
return next(req);
140+
return false;
145141
}
146142

147-
const transferState = inject(TransferState);
143+
return true;
144+
}
148145

149-
const originMap: Record<string, string> | null = inject(HTTP_TRANSFER_CACHE_ORIGIN_MAP, {
150-
optional: true,
151-
});
146+
function getHeadersToInclude(
147+
options: CacheOptions,
148+
requestOptions: HttpTransferCacheOptions | boolean | undefined,
149+
): string[] | undefined {
150+
const {includeHeaders: globalHeaders} = options;
151+
let headersToInclude = globalHeaders;
152+
if (typeof requestOptions === 'object' && requestOptions.includeHeaders) {
153+
// Request-specific config takes precedence over the global config.
154+
headersToInclude = requestOptions.includeHeaders;
155+
}
156+
return headersToInclude;
157+
}
158+
159+
export function retrieveStateFromCache(
160+
req: HttpRequest<unknown>,
161+
options: CacheOptions,
162+
transferState: TransferState,
163+
originMap: Record<string, string> | null,
164+
): HttpResponse<unknown> | null {
165+
const {transferCache: requestOptions} = req;
166+
167+
// In the following situations we do not want to cache the request
168+
if (!shouldCacheRequest(req, options)) {
169+
return null;
170+
}
152171

153172
if (typeof ngServerMode !== 'undefined' && !ngServerMode && originMap) {
154173
throw new RuntimeError(
@@ -168,11 +187,7 @@ export function transferCacheInterceptorFn(
168187
const storeKey = makeCacheKey(req, requestUrl);
169188
const response = transferState.get(storeKey, null);
170189

171-
let headersToInclude = globalOptions.includeHeaders;
172-
if (typeof requestOptions === 'object' && requestOptions.includeHeaders) {
173-
// Request-specific config takes precedence over the global config.
174-
headersToInclude = requestOptions.includeHeaders;
175-
}
190+
const headersToInclude = getHeadersToInclude(options, requestOptions);
176191

177192
if (response) {
178193
const {
@@ -206,15 +221,44 @@ export function transferCacheInterceptorFn(
206221
headers = appendMissingHeadersDetection(req.url, headers, headersToInclude ?? []);
207222
}
208223

209-
return of(
210-
new HttpResponse({
211-
body,
212-
headers,
213-
status,
214-
statusText,
215-
url,
216-
}),
217-
);
224+
return new HttpResponse({
225+
body,
226+
headers,
227+
status,
228+
statusText,
229+
url,
230+
});
231+
}
232+
233+
return null;
234+
}
235+
236+
export function transferCacheInterceptorFn(
237+
req: HttpRequest<unknown>,
238+
next: HttpHandlerFn,
239+
): Observable<HttpEvent<unknown>> {
240+
const options = inject(CACHE_OPTIONS);
241+
const transferState = inject(TransferState);
242+
243+
const originMap = inject(HTTP_TRANSFER_CACHE_ORIGIN_MAP, {optional: true});
244+
245+
const cachedResponse = retrieveStateFromCache(req, options, transferState, originMap);
246+
if (cachedResponse) {
247+
return of(cachedResponse);
248+
}
249+
250+
const {transferCache: requestOptions} = req;
251+
const headersToInclude = getHeadersToInclude(options, requestOptions);
252+
253+
const requestUrl =
254+
typeof ngServerMode !== 'undefined' && ngServerMode && originMap
255+
? mapRequestOriginUrl(req.url, originMap)
256+
: req.url;
257+
const storeKey = makeCacheKey(req, requestUrl);
258+
259+
// In the following situations we do not want to cache the request
260+
if (!shouldCacheRequest(req, options)) {
261+
return next(req);
218262
}
219263

220264
const event$ = next(req);

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

Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ import {
1818
HttpResourceRef,
1919
} from '../index';
2020
import {HttpTestingController, provideHttpClientTesting} from '../testing';
21+
import {withHttpTransferCache} from '../src/transfer_cache';
22+
import {HttpClient} from '../src/client';
2123

2224
describe('httpResource', () => {
2325
beforeEach(() => {
@@ -400,4 +402,59 @@ describe('httpResource', () => {
400402
}
401403
});
402404
});
405+
406+
describe('TransferCache integration', () => {
407+
beforeEach(() => {
408+
TestBed.resetTestingModule();
409+
TestBed.configureTestingModule({
410+
providers: [provideHttpClient(), provideHttpClientTesting(), withHttpTransferCache({})],
411+
});
412+
});
413+
414+
it('should synchronously resolve with a cached value from TransferState', async () => {
415+
globalThis['ngServerMode'] = true;
416+
let requestResolved = false;
417+
TestBed.inject(HttpClient)
418+
.get('/data')
419+
.subscribe(() => (requestResolved = true));
420+
const req = TestBed.inject(HttpTestingController).expectOne('/data');
421+
req.flush([1, 2, 3]);
422+
423+
expect(requestResolved).toBe(true);
424+
425+
// Now switch to client mode
426+
globalThis['ngServerMode'] = false;
427+
428+
// Create httpResource. It should immediately read from TransferState.
429+
const res = httpResource(() => '/data', {injector: TestBed.inject(Injector)});
430+
431+
// It should immediately have the value synchronously and status should be resolved
432+
expect(res.status()).toBe('resolved');
433+
expect(res.hasValue()).toBe(true);
434+
expect(res.value()).toEqual([1, 2, 3]);
435+
436+
// Also no new request should be made
437+
TestBed.inject(HttpTestingController).expectNone('/data');
438+
});
439+
440+
it('should not evaluate the request payload during resource initialization', () => {
441+
let requestEvaluated = false;
442+
const res = httpResource(
443+
() => {
444+
requestEvaluated = true;
445+
return '/data';
446+
},
447+
{injector: TestBed.inject(Injector)},
448+
);
449+
450+
// Request function should NOT be evaluated during initialization
451+
expect(requestEvaluated).toBe(false);
452+
453+
// Read to trigger it
454+
res.status();
455+
456+
// The request should now have been evaluated
457+
expect(requestEvaluated).toBe(true);
458+
});
459+
});
403460
});

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,7 @@ export class ResourceImpl<T, R> extends BaseWritableResource<T> implements Resou
196196
private readonly equal: ValueEqualityFn<T> | undefined,
197197
private readonly debugName: string | undefined,
198198
injector: Injector,
199+
getInitialStream?: (request: R) => Signal<ResourceStreamItem<T>> | undefined,
199200
) {
200201
super(
201202
// Feed a computed signal for the value to `BaseWritableResource`, which will upgrade it to a
@@ -238,15 +239,20 @@ export class ResourceImpl<T, R> extends BaseWritableResource<T> implements Resou
238239
source: this.extRequest,
239240
// Compute the state of the resource given a change in status.
240241
computation: (extRequest, previous) => {
241-
const status = extRequest.request === undefined ? 'idle' : 'loading';
242242
if (!previous) {
243+
const initialStream = getInitialStream?.(extRequest.request as R);
244+
// Clear getInitialStream so it doesn't hold onto memory
245+
getInitialStream = undefined;
246+
const status =
247+
extRequest.request === undefined ? 'idle' : initialStream ? 'resolved' : 'loading';
243248
return {
244249
extRequest,
245250
status,
246251
previousStatus: 'idle',
247-
stream: undefined,
252+
stream: initialStream,
248253
};
249254
} else {
255+
const status = extRequest.request === undefined ? 'idle' : 'loading';
250256
return {
251257
extRequest,
252258
status,

‎packages/core/test/bundling/hydration/bundle.golden_symbols.json‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -476,6 +476,7 @@
476476
"getFactoryDef",
477477
"getFirstLContainer",
478478
"getGlobalLocale",
479+
"getHeadersToInclude",
479480
"getInheritedInjectableDef",
480481
"getInitialLViewFlagsFromDef",
481482
"getInjectFlag",
@@ -722,6 +723,7 @@
722723
"resolveForwardRef",
723724
"retrieveHydrationInfo",
724725
"retrieveHydrationInfoImpl",
726+
"retrieveStateFromCache",
725727
"retrieveTransferredState",
726728
"runAfterLeaveAnimations",
727729
"runEffectsInView",
@@ -770,6 +772,7 @@
770772
"shimHostAttribute",
771773
"shimStylesContent",
772774
"shouldBeIgnoredByZone",
775+
"shouldCacheRequest",
773776
"shouldSearchParent",
774777
"siblingAfter",
775778
"skipTextNodes",

0 commit comments

Comments
 (0)