Skip to content

Commit f14a59c

Browse files
sean-perkinsthetaPCaveryjohnston
authored
fix(react): lifecycle events are removed on page unmount (#28316)
Issue number: N/A --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? <!-- Please describe the current behavior that you are modifying. --> While debugging #28186, Maria and I identified that Ionic's lifecycle event listeners (`ionViewWillEnter`, etc.) were being registered multiple times on the same `.ion-page` element. This resulted in problematic behavior, where a user's implementation of our lifecycle hooks, would execute their callback multiple times. ```ts useIonViewWillEnter(() => { // This is called 2x for every time the `ionViewWillEnter` event is emitted (in React 18, dev mode) console.log('hello world'); }); ``` When the Ionic lifecycle event listeners are registered in React, we bind the scope of the class to the callback function. When removing the event listener we additional use the `.bind` syntax. ```tsx componentDidMount() { element.addEventListener('ionViewWillEnter', this.ionViewWillEnter.bind(this)); } componentWillUnmount() { // This creates a new instance of the function to remove! It doesn't remove the original event listener. element.removeEventListener('ionViewWillEnter', this.ionViewWillEnter.bind(this)); } ``` The `.bind` method returns a new instance of the function. This means in the implementation we are creating a new instance of the function when both adding and removing the event listener - resulting in the `removeEventListener` to never remove the original event listener. This behavior only occurred in React 18 in dev mode, as a result of the mount/unmount behavior running 2x for `useEffect` hooks. ## What is the new behavior? <!-- Please describe the behavior or changes that are being added by this PR. --> - Ionic lifecycle event listeners are removed from element references when they are unmounted. - User's lifecycle callback methods are only invoked once per event emission. |Before|After| |----|----| |<img alt="CleanShot 2023-10-09 at 18 32 08@2x" src="https://github.com/ionic-team/ionic-framework/assets/13732623/53f2ef5d-5900-4a84-b427-fa6c9d35d081">|<img alt="CleanShot 2023-10-09 at 18 29 37@2x" src="https://github.com/ionic-team/ionic-framework/assets/13732623/c8a9a657-a0bf-4d6d-9f21-a41a686de490">| ## Does this introduce a breaking change? - [ ] Yes - [x] No <!-- If this introduces a breaking change, please describe the impact and migration path for existing applications below. --> ## Other information <!-- Any other information that is important to this PR such as screenshots of how the component looks before and after the change. --> --------- Co-authored-by: Maria Hutt <[email protected]> Co-authored-by: Amanda Johnston <[email protected]>
1 parent dcbf451 commit f14a59c

File tree

2 files changed

+38
-16
lines changed

2 files changed

+38
-16
lines changed

packages/react/src/routing/OutletPageManager.tsx

+19-8
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,17 @@ export class OutletPageManager extends React.Component<OutletPageManagerProps> {
2424
super(props);
2525

2626
this.outletIsReady = false;
27+
28+
/**
29+
* This binds the scope of the following methods to the class scope.
30+
* The `.bind` method returns a new function, so we need to assign it
31+
* in the constructor rather than when adding or removing the listeners
32+
* to avoid creating a new function.
33+
*/
34+
this.ionViewWillEnterHandler = this.ionViewWillEnterHandler.bind(this);
35+
this.ionViewDidEnterHandler = this.ionViewDidEnterHandler.bind(this);
36+
this.ionViewWillLeaveHandler = this.ionViewWillLeaveHandler.bind(this);
37+
this.ionViewDidLeaveHandler = this.ionViewDidLeaveHandler.bind(this);
2738
}
2839

2940
componentDidMount() {
@@ -39,19 +50,19 @@ export class OutletPageManager extends React.Component<OutletPageManagerProps> {
3950
});
4051
}
4152

42-
this.ionRouterOutlet.addEventListener('ionViewWillEnter', this.ionViewWillEnterHandler.bind(this));
43-
this.ionRouterOutlet.addEventListener('ionViewDidEnter', this.ionViewDidEnterHandler.bind(this));
44-
this.ionRouterOutlet.addEventListener('ionViewWillLeave', this.ionViewWillLeaveHandler.bind(this));
45-
this.ionRouterOutlet.addEventListener('ionViewDidLeave', this.ionViewDidLeaveHandler.bind(this));
53+
this.ionRouterOutlet.addEventListener('ionViewWillEnter', this.ionViewWillEnterHandler);
54+
this.ionRouterOutlet.addEventListener('ionViewDidEnter', this.ionViewDidEnterHandler);
55+
this.ionRouterOutlet.addEventListener('ionViewWillLeave', this.ionViewWillLeaveHandler);
56+
this.ionRouterOutlet.addEventListener('ionViewDidLeave', this.ionViewDidLeaveHandler);
4657
}
4758
}
4859

4960
componentWillUnmount() {
5061
if (this.ionRouterOutlet) {
51-
this.ionRouterOutlet.removeEventListener('ionViewWillEnter', this.ionViewWillEnterHandler.bind(this));
52-
this.ionRouterOutlet.removeEventListener('ionViewDidEnter', this.ionViewDidEnterHandler.bind(this));
53-
this.ionRouterOutlet.removeEventListener('ionViewWillLeave', this.ionViewWillLeaveHandler.bind(this));
54-
this.ionRouterOutlet.removeEventListener('ionViewDidLeave', this.ionViewDidLeaveHandler.bind(this));
62+
this.ionRouterOutlet.removeEventListener('ionViewWillEnter', this.ionViewWillEnterHandler);
63+
this.ionRouterOutlet.removeEventListener('ionViewDidEnter', this.ionViewDidEnterHandler);
64+
this.ionRouterOutlet.removeEventListener('ionViewWillLeave', this.ionViewWillLeaveHandler);
65+
this.ionRouterOutlet.removeEventListener('ionViewDidLeave', this.ionViewDidLeaveHandler);
5566
}
5667
}
5768

packages/react/src/routing/PageManager.tsx

+19-8
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,17 @@ export class PageManager extends React.PureComponent<PageManagerProps> {
2323
this.ionPageElementRef = React.createRef();
2424
// React refs must be stable (not created inline).
2525
this.stableMergedRefs = mergeRefs(this.ionPageElementRef, this.props.forwardedRef);
26+
27+
/**
28+
* This binds the scope of the following methods to the class scope.
29+
* The `.bind` method returns a new function, so we need to assign it
30+
* in the constructor rather than when adding or removing the listeners
31+
* to avoid creating a new function.
32+
*/
33+
this.ionViewWillEnterHandler = this.ionViewWillEnterHandler.bind(this);
34+
this.ionViewDidEnterHandler = this.ionViewDidEnterHandler.bind(this);
35+
this.ionViewWillLeaveHandler = this.ionViewWillLeaveHandler.bind(this);
36+
this.ionViewDidLeaveHandler = this.ionViewDidLeaveHandler.bind(this);
2637
}
2738

2839
componentDidMount() {
@@ -31,19 +42,19 @@ export class PageManager extends React.PureComponent<PageManagerProps> {
3142
this.ionPageElementRef.current.classList.add('ion-page-invisible');
3243
}
3344
this.context.registerIonPage(this.ionPageElementRef.current, this.props.routeInfo!);
34-
this.ionPageElementRef.current.addEventListener('ionViewWillEnter', this.ionViewWillEnterHandler.bind(this));
35-
this.ionPageElementRef.current.addEventListener('ionViewDidEnter', this.ionViewDidEnterHandler.bind(this));
36-
this.ionPageElementRef.current.addEventListener('ionViewWillLeave', this.ionViewWillLeaveHandler.bind(this));
37-
this.ionPageElementRef.current.addEventListener('ionViewDidLeave', this.ionViewDidLeaveHandler.bind(this));
45+
this.ionPageElementRef.current.addEventListener('ionViewWillEnter', this.ionViewWillEnterHandler);
46+
this.ionPageElementRef.current.addEventListener('ionViewDidEnter', this.ionViewDidEnterHandler);
47+
this.ionPageElementRef.current.addEventListener('ionViewWillLeave', this.ionViewWillLeaveHandler);
48+
this.ionPageElementRef.current.addEventListener('ionViewDidLeave', this.ionViewDidLeaveHandler);
3849
}
3950
}
4051

4152
componentWillUnmount() {
4253
if (this.ionPageElementRef.current) {
43-
this.ionPageElementRef.current.removeEventListener('ionViewWillEnter', this.ionViewWillEnterHandler.bind(this));
44-
this.ionPageElementRef.current.removeEventListener('ionViewDidEnter', this.ionViewDidEnterHandler.bind(this));
45-
this.ionPageElementRef.current.removeEventListener('ionViewWillLeave', this.ionViewWillLeaveHandler.bind(this));
46-
this.ionPageElementRef.current.removeEventListener('ionViewDidLeave', this.ionViewDidLeaveHandler.bind(this));
54+
this.ionPageElementRef.current.removeEventListener('ionViewWillEnter', this.ionViewWillEnterHandler);
55+
this.ionPageElementRef.current.removeEventListener('ionViewDidEnter', this.ionViewDidEnterHandler);
56+
this.ionPageElementRef.current.removeEventListener('ionViewWillLeave', this.ionViewWillLeaveHandler);
57+
this.ionPageElementRef.current.removeEventListener('ionViewDidLeave', this.ionViewDidLeaveHandler);
4758
}
4859
}
4960

0 commit comments

Comments
 (0)