fix(react): improving lifecycle hooks to deal with stale closures, fixes

#19873 (#19874)
ionic-team · Nov 8, 2019 · 5ff786a · 5ff786a
1 parent f0efe4a
commit 5ff786a
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 16 deletions.
diff --git a/packages/react/src/contexts/IonLifeCycleContext.tsx b/packages/react/src/contexts/IonLifeCycleContext.tsx
@@ -22,7 +22,7 @@ export const IonLifeCycleContext = /*@__PURE__*/React.createContext<IonLifeCycle
   ionViewDidLeave: () => { return; },
 });
 
-type LifeCycleCallback = () => void;
+export interface LifeCycleCallback { (): void; id?: number; }
 
 export const DefaultIonLifeCycleContext = class implements IonLifeCycleContextInterface {
 
@@ -33,31 +33,67 @@ export const DefaultIonLifeCycleContext = class implements IonLifeCycleContextIn
   componentCanBeDestroyedCallback?: () => void;
 
   onIonViewWillEnter(callback: LifeCycleCallback) {
-    this.ionViewWillEnterCallbacks.push(callback);
+    if (callback.id) {
+      const index = this.ionViewWillEnterCallbacks.findIndex(x => x.id === callback.id);
+      if (index > -1) {
+        this.ionViewWillEnterCallbacks[index] = callback;
+      } else {
+        this.ionViewWillEnterCallbacks.push(callback);
+      }
+    } else {
+      this.ionViewWillEnterCallbacks.push(callback);
+    }
   }
 
   ionViewWillEnter() {
     this.ionViewWillEnterCallbacks.forEach(cb => cb());
   }
 
   onIonViewDidEnter(callback: LifeCycleCallback) {
-    this.ionViewDidEnterCallbacks.push(callback);
+    if (callback.id) {
+      const index = this.ionViewDidEnterCallbacks.findIndex(x => x.id === callback.id);
+      if (index > -1) {
+        this.ionViewDidEnterCallbacks[index] = callback;
+      } else {
+        this.ionViewDidEnterCallbacks.push(callback);
+      }
+    } else {
+      this.ionViewDidEnterCallbacks.push(callback);
+    }
   }
 
   ionViewDidEnter() {
     this.ionViewDidEnterCallbacks.forEach(cb => cb());
   }
 
   onIonViewWillLeave(callback: LifeCycleCallback) {
-    this.ionViewWillLeaveCallbacks.push(callback);
+    if (callback.id) {
+      const index = this.ionViewWillLeaveCallbacks.findIndex(x => x.id === callback.id);
+      if (index > -1) {
+        this.ionViewWillLeaveCallbacks[index] = callback;
+      } else {
+        this.ionViewWillLeaveCallbacks.push(callback);
+      }
+    } else {
+      this.ionViewWillLeaveCallbacks.push(callback);
+    }
   }
 
   ionViewWillLeave() {
     this.ionViewWillLeaveCallbacks.forEach(cb => cb());
   }
 
   onIonViewDidLeave(callback: LifeCycleCallback) {
-    this.ionViewDidLeaveCallbacks.push(callback);
+    if (callback.id) {
+      const index = this.ionViewDidLeaveCallbacks.findIndex(x => x.id === callback.id);
+      if (index > -1) {
+        this.ionViewDidLeaveCallbacks[index] = callback;
+      } else {
+        this.ionViewDidLeaveCallbacks.push(callback);
+      }
+    } else {
+      this.ionViewDidLeaveCallbacks.push(callback);
+    }
   }
 
   ionViewDidLeave() {

diff --git a/packages/react/src/lifecycle/hooks.ts b/packages/react/src/lifecycle/hooks.ts
@@ -1,31 +1,43 @@
-import { useContext, useEffect } from 'react';
+import { useContext, useEffect, useRef } from 'react';
 
-import { IonLifeCycleContext } from '../contexts/IonLifeCycleContext';
+import { IonLifeCycleContext, LifeCycleCallback } from '../contexts/IonLifeCycleContext';
 
-export const useIonViewWillEnter = (callback: () => void) => {
+export const useIonViewWillEnter = (callback: LifeCycleCallback, deps: any[] = []) => {
   const context = useContext(IonLifeCycleContext);
+  const id = useRef<number | undefined>();
+  id.current = id.current || Math.floor(Math.random() * 1000000);
   useEffect(() => {
+    callback.id = id.current!;
     context.onIonViewWillEnter(callback);
-  }, []);
+  }, deps);
 };
 
-export const useIonViewDidEnter = (callback: () => void) => {
+export const useIonViewDidEnter = (callback: LifeCycleCallback, deps: any[] = []) => {
   const context = useContext(IonLifeCycleContext);
+  const id = useRef<number | undefined>();
+  id.current = id.current || Math.floor(Math.random() * 1000000);
   useEffect(() => {
+    callback.id = id.current!;
     context.onIonViewDidEnter(callback);
-  }, []);
+  }, deps);
 };
 
-export const useIonViewWillLeave = (callback: () => void) => {
+export const useIonViewWillLeave = (callback: LifeCycleCallback, deps: any[] = []) => {
   const context = useContext(IonLifeCycleContext);
+  const id = useRef<number | undefined>();
+  id.current = id.current || Math.floor(Math.random() * 1000000);
   useEffect(() => {
+    callback.id = id.current!;
     context.onIonViewWillLeave(callback);
-  }, []);
+  }, deps);
 };
 
-export const useIonViewDidLeave = (callback: () => void) => {
+export const useIonViewDidLeave = (callback: LifeCycleCallback, deps: any[] = []) => {
   const context = useContext(IonLifeCycleContext);
+  const id = useRef<number | undefined>();
+  id.current = id.current || Math.floor(Math.random() * 1000000);
   useEffect(() => {
+    callback.id = id.current!;
     context.onIonViewDidLeave(callback);
-  }, []);
+  }, deps);
 };
diff --git a/packages/react/tslint.json b/packages/react/tslint.json
@@ -19,7 +19,7 @@
     "no-invalid-template-strings": true,
     "ban-export-const-enum": true,
     "only-arrow-functions": true,
-    "strict-boolean-conditions": [true, "allow-null-union", "allow-undefined-union", "allow-boolean-or-undefined", "allow-string"],
+    "strict-boolean-conditions": [false],
     "jsx-key": false,
     "jsx-self-close": false,
     "jsx-curly-spacing": [true, "never"],