Shouldn't TransferState.set be called only on server side?

[zip-fa]

Which @angular/* package(s) are the source of the bug?

core

Is this a regression?

No

Description

Hi.

I noticed that this piece of code in packages/common/http/src/transfer_cache.ts gets called on client side:

return next(req).pipe(
    tap((event: HttpEvent<unknown>) => {
      if (event instanceof HttpResponse) {
        transferState.set<TransferHttpResponse>(storeKey, {
          [BODY]: event.body,
          [HEADERS]: getFilteredHeaders(event.headers, headersToInclude),
          [STATUS]: event.status,
          [STATUS_TEXT]: event.statusText,
          [URL]: event.url || '',
          [RESPONSE_TYPE]: req.responseType,
        });
      }
    }),
  );

Shouldn't this only be called only on server side? It wastes some memory on client's device

[alan-agius4]

Technically this could only be called on the client side when;

• The cache is in active, meaning that application has not been stabilised yet.
• The request was not already performed on the server.

In general, I'd say that this is rather quite uncommon, although I don't have anything against adding the additional check.

[dchicurel]

Adding +1 as we just found this issue. For some reasons, we needed to have the app deployed either as CSR as SSR and found this issue. Transfer state seems to be working just as a store in client mode, so it's caching the current actual client requests.

Editing to add a bit more detail, as the issue I was mentioned is very similar but not exactly the same as the original. Basically, I think http cache (with transferState) shouldn't be enabled at all when the app is deployed only in CSR ? (if there's not transferState)

[dchicurel]

We just found that transfer cache should only work during App bootstrapping and it's being disabled after the app is stable. That was well thought. I didn't noticed in your comment @alan-agius4 when you mentioned about the app not "stabilised" 😊

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

Lines 264 to 274 in 5bd188a

	provide: APP_BOOTSTRAP_LISTENER,
	multi: true,
	useFactory: () => {
	const appRef = inject(ApplicationRef);
	const cacheState = inject(CACHE_OPTIONS);
	return () => {
	whenStable(appRef).then(() => {
	cacheState.isCacheActive = false;
	});
	};

Our problem was that, actually, the App was not being Stable! We fixed that and confirmed that cache is disabled after the app becomes stable (first time).
Now it makes sense your comment also that this (original) issue would be an uncommon case.

[alan-agius4]

Closed via #55012