From eec7cc4ea772a07e582c25f6d5b7ce4891a88af7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lucas=20Lef=C3=A8vre=20=28lul=29?= <lul@odoo.com>
Date: Fri, 29 Sep 2023 11:16:13 +0200
Subject: [PATCH] [FIX] hooks: remove useEffect type circular dependency

`useEffect` type is not compatible with later versions of Typescript.
(I tried with Typescript 5.2, I didn't check which specific
version had the breaking change)

This prevents other projects using owl (such as o-spreadsheet) to
upgrade their own version of Typescript.

`<T extends [...T]>` raises the two following errors:

`Type parameter 'T' has a circular constraint.ts(2313)`
`A rest element type must be an array type.`
---
 src/runtime/hooks.ts           | 10 +++----
 tests/components/hooks.test.ts | 52 ++++++++++++++++++++++++++++++++++
 2 files changed, 57 insertions(+), 5 deletions(-)

diff --git a/src/runtime/hooks.ts b/src/runtime/hooks.ts
index beac0c28..2741b06a 100644
--- a/src/runtime/hooks.ts
+++ b/src/runtime/hooks.ts
@@ -59,7 +59,7 @@ export function useChildSubEnv(envExtension: Env) {
 // useEffect
 // -----------------------------------------------------------------------------
 
-type EffectDeps<T extends any[]> = T | (T extends [...infer H, never] ? EffectDeps<H> : never);
+type EffectDeps<T extends unknown[]> = T | (T extends [...infer H, never] ? EffectDeps<H> : never);
 
 /**
  * @template T
@@ -67,7 +67,7 @@ type EffectDeps<T extends any[]> = T | (T extends [...infer H, never] ? EffectDe
  * @returns {void|(()=>void)} a cleanup function that reverses the side
  *      effects of the effect callback.
  */
-type Effect<T extends [...T]> = (...dependencies: EffectDeps<T>) => void | (() => void);
+type Effect<T extends unknown[]> = (...dependencies: EffectDeps<T>) => void | (() => void);
 
 /**
  * This hook will run a callback when a component is mounted and patched, and
@@ -76,15 +76,15 @@ type Effect<T extends [...T]> = (...dependencies: EffectDeps<T>) => void | (() =
  *
  * @template T
  * @param {Effect<T>} effect the effect to run on component mount and/or patch
- * @param {()=>T} [computeDependencies=()=>[NaN]] a callback to compute
+ * @param {()=>[...T]} [computeDependencies=()=>[NaN]] a callback to compute
  *      dependencies that will decide if the effect needs to be cleaned up and
  *      run again. If the dependencies did not change, the effect will not run
  *      again. The default value returns an array containing only NaN because
  *      NaN !== NaN, which will cause the effect to rerun on every patch.
  */
-export function useEffect<T extends [...T]>(
+export function useEffect<T extends unknown[]>(
   effect: Effect<T>,
-  computeDependencies: () => T = () => [NaN] as never
+  computeDependencies: () => [...T] = () => [NaN] as never
 ) {
   let cleanup: (() => void) | void;
   let dependencies: T;
diff --git a/tests/components/hooks.test.ts b/tests/components/hooks.test.ts
index 7b72e265..45fdeaf7 100644
--- a/tests/components/hooks.test.ts
+++ b/tests/components/hooks.test.ts
@@ -641,6 +641,56 @@ describe("hooks", () => {
       ]);
     });
 
+    test("effect types are inferred from dependencies", async () => {
+      // @ts-ignore (declared but never used)
+      // eslint-disable-next-line @typescript-eslint/no-unused-vars
+      class MyComponent extends Component {
+        static template = xml`<div/>`;
+
+        setup() {
+          useEffect(
+            (a, b) => {
+              expectType<number>(a);
+              expectType<string>(b);
+            },
+            () => [3, "hello"]
+          );
+        }
+      }
+    });
+
+    test("effect type allows an effect with partial dependencies parameters", async () => {
+      // @ts-ignore (declared but never used)
+      // eslint-disable-next-line @typescript-eslint/no-unused-vars
+      class MyComponent extends Component {
+        static template = xml`<div/>`;
+
+        setup() {
+          useEffect(
+            (a) => {
+              expectType<number>(a);
+            },
+            () => [3, "hello"]
+          );
+        }
+      }
+    });
+
+    test("effect type allows an effect with no dependency parameter", async () => {
+      // @ts-ignore (declared but never used)
+      // eslint-disable-next-line @typescript-eslint/no-unused-vars
+      class MyComponent extends Component {
+        static template = xml`<div/>`;
+
+        setup() {
+          useEffect(
+            () => {},
+            () => [3, "hello"]
+          );
+        }
+      }
+    });
+
     test("properly behaves when the effect function throws", async () => {
       let originalconsoleError = console.error;
       let originalconsoleWarn = console.warn;
@@ -673,3 +723,5 @@ describe("hooks", () => {
     });
   });
 });
+
+function expectType<T>(t: T) {}