From 23e4da430ab9eecb9ce9102bfe5d4a3de4e34443 Mon Sep 17 00:00:00 2001
From: Matthieu Riegler <kyro38@gmail.com>
Date: Mon, 24 Mar 2025 18:31:20 +0100
Subject: [PATCH] refactor(http): remove non reactive signature from
 `httpResource`. (#60537)

The non reactive usage ``httpResource(\`http://test/${myId()}\`` gave the false impression that it was reactive when the signal was invoked. We prevent any possibility of oversight by removing the non reactive signature.

PR Close #60537
---
 goldens/public-api/common/http/index.api.md | 32 +++++------
 packages/common/http/src/resource.ts        | 59 +++++++++------------
 packages/common/http/test/resource_spec.ts  | 26 ++++-----
 3 files changed, 53 insertions(+), 64 deletions(-)

diff --git a/goldens/public-api/common/http/index.api.md b/goldens/public-api/common/http/index.api.md
index 80cf18133c7..d038f7032b2 100644
--- a/goldens/public-api/common/http/index.api.md
+++ b/goldens/public-api/common/http/index.api.md
@@ -1678,43 +1678,43 @@ export const httpResource: HttpResourceFn;
 
 // @public
 export interface HttpResourceFn {
-    <TResult = unknown>(url: string | (() => string | undefined), options: HttpResourceOptions<TResult, unknown> & {
+    <TResult = unknown>(url: () => string | undefined, options: HttpResourceOptions<TResult, unknown> & {
         defaultValue: NoInfer<TResult>;
     }): HttpResourceRef<TResult>;
-    <TResult = unknown>(url: string | (() => string | undefined), options?: HttpResourceOptions<TResult, unknown>): HttpResourceRef<TResult | undefined>;
-    <TResult = unknown>(request: HttpResourceRequest | (() => HttpResourceRequest | undefined), options: HttpResourceOptions<TResult, unknown> & {
+    <TResult = unknown>(url: () => string | undefined, options?: HttpResourceOptions<TResult, unknown>): HttpResourceRef<TResult | undefined>;
+    <TResult = unknown>(request: () => HttpResourceRequest | undefined, options: HttpResourceOptions<TResult, unknown> & {
         defaultValue: NoInfer<TResult>;
     }): HttpResourceRef<TResult>;
-    <TResult = unknown>(request: HttpResourceRequest | (() => HttpResourceRequest | undefined), options?: HttpResourceOptions<TResult, unknown>): HttpResourceRef<TResult | undefined>;
+    <TResult = unknown>(request: () => HttpResourceRequest | undefined, options?: HttpResourceOptions<TResult, unknown>): HttpResourceRef<TResult | undefined>;
     arrayBuffer: {
-        <TResult = ArrayBuffer>(url: string | (() => string | undefined), options: HttpResourceOptions<TResult, ArrayBuffer> & {
+        <TResult = ArrayBuffer>(url: () => string | undefined, options: HttpResourceOptions<TResult, ArrayBuffer> & {
             defaultValue: NoInfer<TResult>;
         }): HttpResourceRef<TResult>;
-        <TResult = ArrayBuffer>(url: string | (() => string | undefined), options?: HttpResourceOptions<TResult, ArrayBuffer>): HttpResourceRef<TResult | undefined>;
-        <TResult = ArrayBuffer>(request: HttpResourceRequest | (() => HttpResourceRequest | undefined), options: HttpResourceOptions<TResult, ArrayBuffer> & {
+        <TResult = ArrayBuffer>(url: () => string | undefined, options?: HttpResourceOptions<TResult, ArrayBuffer>): HttpResourceRef<TResult | undefined>;
+        <TResult = ArrayBuffer>(request: () => HttpResourceRequest | undefined, options: HttpResourceOptions<TResult, ArrayBuffer> & {
             defaultValue: NoInfer<TResult>;
         }): HttpResourceRef<TResult>;
-        <TResult = ArrayBuffer>(request: HttpResourceRequest | (() => HttpResourceRequest | undefined), options?: HttpResourceOptions<TResult, ArrayBuffer>): HttpResourceRef<TResult | undefined>;
+        <TResult = ArrayBuffer>(request: () => HttpResourceRequest | undefined, options?: HttpResourceOptions<TResult, ArrayBuffer>): HttpResourceRef<TResult | undefined>;
     };
     blob: {
-        <TResult = Blob>(url: string | (() => string | undefined), options: HttpResourceOptions<TResult, Blob> & {
+        <TResult = Blob>(url: () => string | undefined, options: HttpResourceOptions<TResult, Blob> & {
             defaultValue: NoInfer<TResult>;
         }): HttpResourceRef<TResult>;
-        <TResult = Blob>(url: string | (() => string | undefined), options?: HttpResourceOptions<TResult, Blob>): HttpResourceRef<TResult | undefined>;
-        <TResult = Blob>(request: HttpResourceRequest | (() => HttpResourceRequest | undefined), options: HttpResourceOptions<TResult, Blob> & {
+        <TResult = Blob>(url: () => string | undefined, options?: HttpResourceOptions<TResult, Blob>): HttpResourceRef<TResult | undefined>;
+        <TResult = Blob>(request: () => HttpResourceRequest | undefined, options: HttpResourceOptions<TResult, Blob> & {
             defaultValue: NoInfer<TResult>;
         }): HttpResourceRef<TResult>;
-        <TResult = Blob>(request: HttpResourceRequest | (() => HttpResourceRequest | undefined), options?: HttpResourceOptions<TResult, Blob>): HttpResourceRef<TResult | undefined>;
+        <TResult = Blob>(request: () => HttpResourceRequest | undefined, options?: HttpResourceOptions<TResult, Blob>): HttpResourceRef<TResult | undefined>;
     };
     text: {
-        <TResult = string>(url: string | (() => string | undefined), options: HttpResourceOptions<TResult, string> & {
+        <TResult = string>(url: () => string | undefined, options: HttpResourceOptions<TResult, string> & {
             defaultValue: NoInfer<TResult>;
         }): HttpResourceRef<TResult>;
-        <TResult = string>(url: string | (() => string | undefined), options?: HttpResourceOptions<TResult, string>): HttpResourceRef<TResult | undefined>;
-        <TResult = string>(request: HttpResourceRequest | (() => HttpResourceRequest | undefined), options: HttpResourceOptions<TResult, string> & {
+        <TResult = string>(url: () => string | undefined, options?: HttpResourceOptions<TResult, string>): HttpResourceRef<TResult | undefined>;
+        <TResult = string>(request: () => HttpResourceRequest | undefined, options: HttpResourceOptions<TResult, string> & {
             defaultValue: NoInfer<TResult>;
         }): HttpResourceRef<TResult>;
-        <TResult = string>(request: HttpResourceRequest | (() => HttpResourceRequest | undefined), options?: HttpResourceOptions<TResult, string>): HttpResourceRef<TResult | undefined>;
+        <TResult = string>(request: () => HttpResourceRequest | undefined, options?: HttpResourceOptions<TResult, string>): HttpResourceRef<TResult | undefined>;
     };
 }
 
diff --git a/packages/common/http/src/resource.ts b/packages/common/http/src/resource.ts
index 69599b817e4..e0feb25c3f3 100644
--- a/packages/common/http/src/resource.ts
+++ b/packages/common/http/src/resource.ts
@@ -41,8 +41,7 @@ export interface HttpResourceFn {
   /**
    * Create a `Resource` that fetches data with an HTTP GET request to the given URL.
    *
-   * If a reactive function is passed for the URL, the resource will update when the URL changes via
-   * signals.
+   * The resource will update when the URL changes via signals.
    *
    * Uses `HttpClient` to make requests and supports interceptors, testing, and the other features
    * of the `HttpClient` API. Data is parsed as JSON by default - use a sub-function of
@@ -51,15 +50,14 @@ export interface HttpResourceFn {
    * @experimental
    */
   <TResult = unknown>(
-    url: string | (() => string | undefined),
+    url: () => string | undefined,
     options: HttpResourceOptions<TResult, unknown> & {defaultValue: NoInfer<TResult>},
   ): HttpResourceRef<TResult>;
 
   /**
    * Create a `Resource` that fetches data with an HTTP GET request to the given URL.
    *
-   * If a reactive function is passed for the URL, the resource will update when the URL changes via
-   * signals.
+   * The resource will update when the URL changes via signals.
    *
    * Uses `HttpClient` to make requests and supports interceptors, testing, and the other features
    * of the `HttpClient` API. Data is parsed as JSON by default - use a sub-function of
@@ -68,15 +66,14 @@ export interface HttpResourceFn {
    * @experimental
    */
   <TResult = unknown>(
-    url: string | (() => string | undefined),
+    url: () => string | undefined,
     options?: HttpResourceOptions<TResult, unknown>,
   ): HttpResourceRef<TResult | undefined>;
 
   /**
    * Create a `Resource` that fetches data with the configured HTTP request.
    *
-   * If a reactive function is passed for the request, the resource will update when the request
-   * changes via signals.
+   * The resource will update when the request changes via signals.
    *
    * Uses `HttpClient` to make requests and supports interceptors, testing, and the other features
    * of the `HttpClient` API. Data is parsed as JSON by default - use a sub-function of
@@ -85,15 +82,14 @@ export interface HttpResourceFn {
    * @experimental
    */
   <TResult = unknown>(
-    request: HttpResourceRequest | (() => HttpResourceRequest | undefined),
+    request: () => HttpResourceRequest | undefined,
     options: HttpResourceOptions<TResult, unknown> & {defaultValue: NoInfer<TResult>},
   ): HttpResourceRef<TResult>;
 
   /**
    * Create a `Resource` that fetches data with the configured HTTP request.
    *
-   * If a reactive function is passed for the request, the resource will update when the request
-   * changes via signals.
+   * The resource will update when the request changes via signals.
    *
    * Uses `HttpClient` to make requests and supports interceptors, testing, and the other features
    * of the `HttpClient` API. Data is parsed as JSON by default - use a sub-function of
@@ -102,15 +98,14 @@ export interface HttpResourceFn {
    * @experimental
    */
   <TResult = unknown>(
-    request: HttpResourceRequest | (() => HttpResourceRequest | undefined),
+    request: () => HttpResourceRequest | undefined,
     options?: HttpResourceOptions<TResult, unknown>,
   ): HttpResourceRef<TResult | undefined>;
 
   /**
    * Create a `Resource` that fetches data with the configured HTTP request.
    *
-   * If a reactive function is passed for the URL or request, the resource will update when the
-   * URL or request changes via signals.
+   * The resource will update when the URL or request changes via signals.
    *
    * Uses `HttpClient` to make requests and supports interceptors, testing, and the other features
    * of the `HttpClient` API. Data is parsed into an `ArrayBuffer`.
@@ -119,22 +114,22 @@ export interface HttpResourceFn {
    */
   arrayBuffer: {
     <TResult = ArrayBuffer>(
-      url: string | (() => string | undefined),
+      url: () => string | undefined,
       options: HttpResourceOptions<TResult, ArrayBuffer> & {defaultValue: NoInfer<TResult>},
     ): HttpResourceRef<TResult>;
 
     <TResult = ArrayBuffer>(
-      url: string | (() => string | undefined),
+      url: () => string | undefined,
       options?: HttpResourceOptions<TResult, ArrayBuffer>,
     ): HttpResourceRef<TResult | undefined>;
 
     <TResult = ArrayBuffer>(
-      request: HttpResourceRequest | (() => HttpResourceRequest | undefined),
+      request: () => HttpResourceRequest | undefined,
       options: HttpResourceOptions<TResult, ArrayBuffer> & {defaultValue: NoInfer<TResult>},
     ): HttpResourceRef<TResult>;
 
     <TResult = ArrayBuffer>(
-      request: HttpResourceRequest | (() => HttpResourceRequest | undefined),
+      request: () => HttpResourceRequest | undefined,
       options?: HttpResourceOptions<TResult, ArrayBuffer>,
     ): HttpResourceRef<TResult | undefined>;
   };
@@ -142,8 +137,7 @@ export interface HttpResourceFn {
   /**
    * Create a `Resource` that fetches data with the configured HTTP request.
    *
-   * If a reactive function is passed for the URL or request, the resource will update when the
-   * URL or request changes via signals.
+   * The resource will update when the URL or request changes via signals.
    *
    * Uses `HttpClient` to make requests and supports interceptors, testing, and the other features
    * of the `HttpClient` API. Data is parsed into a `Blob`.
@@ -152,22 +146,22 @@ export interface HttpResourceFn {
    */
   blob: {
     <TResult = Blob>(
-      url: string | (() => string | undefined),
+      url: () => string | undefined,
       options: HttpResourceOptions<TResult, Blob> & {defaultValue: NoInfer<TResult>},
     ): HttpResourceRef<TResult>;
 
     <TResult = Blob>(
-      url: string | (() => string | undefined),
+      url: () => string | undefined,
       options?: HttpResourceOptions<TResult, Blob>,
     ): HttpResourceRef<TResult | undefined>;
 
     <TResult = Blob>(
-      request: HttpResourceRequest | (() => HttpResourceRequest | undefined),
+      request: () => HttpResourceRequest | undefined,
       options: HttpResourceOptions<TResult, Blob> & {defaultValue: NoInfer<TResult>},
     ): HttpResourceRef<TResult>;
 
     <TResult = Blob>(
-      request: HttpResourceRequest | (() => HttpResourceRequest | undefined),
+      request: () => HttpResourceRequest | undefined,
       options?: HttpResourceOptions<TResult, Blob>,
     ): HttpResourceRef<TResult | undefined>;
   };
@@ -175,8 +169,7 @@ export interface HttpResourceFn {
   /**
    * Create a `Resource` that fetches data with the configured HTTP request.
    *
-   * If a reactive function is passed for the URL or request, the resource will update when the
-   * URL or request changes via signals.
+   * The resource will update when the URL or request changes via signals.
    *
    * Uses `HttpClient` to make requests and supports interceptors, testing, and the other features
    * of the `HttpClient` API. Data is parsed as a `string`.
@@ -185,22 +178,22 @@ export interface HttpResourceFn {
    */
   text: {
     <TResult = string>(
-      url: string | (() => string | undefined),
+      url: () => string | undefined,
       options: HttpResourceOptions<TResult, string> & {defaultValue: NoInfer<TResult>},
     ): HttpResourceRef<TResult>;
 
     <TResult = string>(
-      url: string | (() => string | undefined),
+      url: () => string | undefined,
       options?: HttpResourceOptions<TResult, string>,
     ): HttpResourceRef<TResult | undefined>;
 
     <TResult = string>(
-      request: HttpResourceRequest | (() => HttpResourceRequest | undefined),
+      request: () => HttpResourceRequest | undefined,
       options: HttpResourceOptions<TResult, string> & {defaultValue: NoInfer<TResult>},
     ): HttpResourceRef<TResult>;
 
     <TResult = string>(
-      request: HttpResourceRequest | (() => HttpResourceRequest | undefined),
+      request: () => HttpResourceRequest | undefined,
       options?: HttpResourceOptions<TResult, string>,
     ): HttpResourceRef<TResult | undefined>;
   };
@@ -223,11 +216,7 @@ export const httpResource: HttpResourceFn = (() => {
   return jsonFn;
 })();
 
-type RawRequestType =
-  | string
-  | (() => string | undefined)
-  | HttpResourceRequest
-  | (() => HttpResourceRequest | undefined);
+type RawRequestType = (() => string | undefined) | (() => HttpResourceRequest | undefined);
 
 function makeHttpResourceFn<TRaw>(responseType: 'arraybuffer' | 'blob' | 'json' | 'text') {
   return function httpResourceRef<TResult = TRaw>(
diff --git a/packages/common/http/test/resource_spec.ts b/packages/common/http/test/resource_spec.ts
index cce24a560db..22c5d7e666f 100644
--- a/packages/common/http/test/resource_spec.ts
+++ b/packages/common/http/test/resource_spec.ts
@@ -24,7 +24,7 @@ describe('httpResource', () => {
 
   it('should send a basic request', async () => {
     const backend = TestBed.inject(HttpTestingController);
-    const res = httpResource('/data', {injector: TestBed.inject(Injector)});
+    const res = httpResource(() => '/data', {injector: TestBed.inject(Injector)});
     TestBed.flushEffects();
     const req = backend.expectOne('/data');
     req.flush([]);
@@ -79,7 +79,7 @@ describe('httpResource', () => {
   it('should support the suite of HttpRequest APIs', async () => {
     const backend = TestBed.inject(HttpTestingController);
     const res = httpResource(
-      {
+      () => ({
         url: '/data',
         method: 'POST',
         body: {message: 'Hello, backend!'},
@@ -90,7 +90,7 @@ describe('httpResource', () => {
           'fast': 'yes',
         },
         withCredentials: true,
-      },
+      }),
       {injector: TestBed.inject(Injector)},
     );
     TestBed.flushEffects();
@@ -108,7 +108,7 @@ describe('httpResource', () => {
 
   it('should return response headers & status when resolved', async () => {
     const backend = TestBed.inject(HttpTestingController);
-    const res = httpResource('/data', {injector: TestBed.inject(Injector)});
+    const res = httpResource(() => '/data', {injector: TestBed.inject(Injector)});
     TestBed.flushEffects();
     const req = backend.expectOne('/data');
     req.flush([], {
@@ -125,10 +125,10 @@ describe('httpResource', () => {
   it('should support progress events', async () => {
     const backend = TestBed.inject(HttpTestingController);
     const res = httpResource(
-      {
+      () => ({
         url: '/data',
         reportProgress: true,
-      },
+      }),
       {injector: TestBed.inject(Injector)},
     );
     TestBed.flushEffects();
@@ -188,10 +188,10 @@ describe('httpResource', () => {
   it('should allow mapping data to an arbitrary type', async () => {
     const backend = TestBed.inject(HttpTestingController);
     const res = httpResource(
-      {
+      () => ({
         url: '/data',
         reportProgress: true,
-      },
+      }),
       {
         injector: TestBed.inject(Injector),
         parse: (value) => JSON.stringify(value),
@@ -207,7 +207,7 @@ describe('httpResource', () => {
 
   it('should allow defining an equality function', async () => {
     const backend = TestBed.inject(HttpTestingController);
-    const res = httpResource<number>('/data', {
+    const res = httpResource<number>(() => '/data', {
       injector: TestBed.inject(Injector),
       equal: (_a, _b) => true,
     });
@@ -225,10 +225,10 @@ describe('httpResource', () => {
   it('should support text responses', async () => {
     const backend = TestBed.inject(HttpTestingController);
     const res = httpResource.text(
-      {
+      () => ({
         url: '/data',
         reportProgress: true,
-      },
+      }),
       {injector: TestBed.inject(Injector)},
     );
     TestBed.flushEffects();
@@ -242,10 +242,10 @@ describe('httpResource', () => {
   it('should support ArrayBuffer responses', async () => {
     const backend = TestBed.inject(HttpTestingController);
     const res = httpResource.arrayBuffer(
-      {
+      () => ({
         url: '/data',
         reportProgress: true,
-      },
+      }),
       {injector: TestBed.inject(Injector)},
     );
     TestBed.flushEffects();