javascript/ionic-framework
1
0
Fork 0
mirror of https://github.com/ionic-team/ionic-framework.git synced 2026-03-13 10:22:08 +08:00
Code Issues Packages Projects Releases Wiki Activity

fix(react): lifecycle events are removed on page unmount (#28316)

Browse Source 
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 <maria@ionic.io>
Co-authored-by: Amanda Johnston <90629384+amandaejohnston@users.noreply.github.com>
This commit is contained in:
Sean Perkins
2023-10-12 11:02:21 -04:00
committed by GitHub
parent dcbf45101f
commit f14a59c5e0
2 changed files with 38 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files

27
packages/react/src/routing/OutletPageManager.tsx
View File

@@ -24,6 +24,17 @@ export class OutletPageManager extends React.Component<OutletPageManagerProps> {
super(props);
this.outletIsReady = false;
/**
* This binds the scope of the following methods to the class scope.
* The `.bind` method returns a new function, so we need to assign it
* in the constructor rather than when adding or removing the listeners
* to avoid creating a new function.
*/
this.ionViewWillEnterHandler = this.ionViewWillEnterHandler.bind(this);
this.ionViewDidEnterHandler = this.ionViewDidEnterHandler.bind(this);
this.ionViewWillLeaveHandler = this.ionViewWillLeaveHandler.bind(this);
this.ionViewDidLeaveHandler = this.ionViewDidLeaveHandler.bind(this);
}
componentDidMount() {
@@ -39,19 +50,19 @@ export class OutletPageManager extends React.Component<OutletPageManagerProps> {
});
}
this.ionRouterOutlet.addEventListener('ionViewWillEnter', this.ionViewWillEnterHandler.bind(this));
this.ionRouterOutlet.addEventListener('ionViewDidEnter', this.ionViewDidEnterHandler.bind(this));
this.ionRouterOutlet.addEventListener('ionViewWillLeave', this.ionViewWillLeaveHandler.bind(this));
this.ionRouterOutlet.addEventListener('ionViewDidLeave', this.ionViewDidLeaveHandler.bind(this));
this.ionRouterOutlet.addEventListener('ionViewWillEnter', this.ionViewWillEnterHandler);
this.ionRouterOutlet.addEventListener('ionViewDidEnter', this.ionViewDidEnterHandler);
this.ionRouterOutlet.addEventListener('ionViewWillLeave', this.ionViewWillLeaveHandler);
this.ionRouterOutlet.addEventListener('ionViewDidLeave', this.ionViewDidLeaveHandler);
}
}
componentWillUnmount() {
if (this.ionRouterOutlet) {
this.ionRouterOutlet.removeEventListener('ionViewWillEnter', this.ionViewWillEnterHandler.bind(this));
this.ionRouterOutlet.removeEventListener('ionViewDidEnter', this.ionViewDidEnterHandler.bind(this));
this.ionRouterOutlet.removeEventListener('ionViewWillLeave', this.ionViewWillLeaveHandler.bind(this));
this.ionRouterOutlet.removeEventListener('ionViewDidLeave', this.ionViewDidLeaveHandler.bind(this));
this.ionRouterOutlet.removeEventListener('ionViewWillEnter', this.ionViewWillEnterHandler);
this.ionRouterOutlet.removeEventListener('ionViewDidEnter', this.ionViewDidEnterHandler);
this.ionRouterOutlet.removeEventListener('ionViewWillLeave', this.ionViewWillLeaveHandler);
this.ionRouterOutlet.removeEventListener('ionViewDidLeave', this.ionViewDidLeaveHandler);
}
}

27
packages/react/src/routing/PageManager.tsx
View File

@@ -23,6 +23,17 @@ export class PageManager extends React.PureComponent<PageManagerProps> {
this.ionPageElementRef = React.createRef();
// React refs must be stable (not created inline).
this.stableMergedRefs = mergeRefs(this.ionPageElementRef, this.props.forwardedRef);
/**
* This binds the scope of the following methods to the class scope.
* The `.bind` method returns a new function, so we need to assign it
* in the constructor rather than when adding or removing the listeners
* to avoid creating a new function.
*/
this.ionViewWillEnterHandler = this.ionViewWillEnterHandler.bind(this);
this.ionViewDidEnterHandler = this.ionViewDidEnterHandler.bind(this);
this.ionViewWillLeaveHandler = this.ionViewWillLeaveHandler.bind(this);
this.ionViewDidLeaveHandler = this.ionViewDidLeaveHandler.bind(this);
}
componentDidMount() {
@@ -31,19 +42,19 @@ export class PageManager extends React.PureComponent<PageManagerProps> {
this.ionPageElementRef.current.classList.add('ion-page-invisible');
}
this.context.registerIonPage(this.ionPageElementRef.current, this.props.routeInfo!);
this.ionPageElementRef.current.addEventListener('ionViewWillEnter', this.ionViewWillEnterHandler.bind(this));
this.ionPageElementRef.current.addEventListener('ionViewDidEnter', this.ionViewDidEnterHandler.bind(this));
this.ionPageElementRef.current.addEventListener('ionViewWillLeave', this.ionViewWillLeaveHandler.bind(this));
this.ionPageElementRef.current.addEventListener('ionViewDidLeave', this.ionViewDidLeaveHandler.bind(this));
this.ionPageElementRef.current.addEventListener('ionViewWillEnter', this.ionViewWillEnterHandler);
this.ionPageElementRef.current.addEventListener('ionViewDidEnter', this.ionViewDidEnterHandler);
this.ionPageElementRef.current.addEventListener('ionViewWillLeave', this.ionViewWillLeaveHandler);
this.ionPageElementRef.current.addEventListener('ionViewDidLeave', this.ionViewDidLeaveHandler);
}
}
componentWillUnmount() {
if (this.ionPageElementRef.current) {
this.ionPageElementRef.current.removeEventListener('ionViewWillEnter', this.ionViewWillEnterHandler.bind(this));
this.ionPageElementRef.current.removeEventListener('ionViewDidEnter', this.ionViewDidEnterHandler.bind(this));
this.ionPageElementRef.current.removeEventListener('ionViewWillLeave', this.ionViewWillLeaveHandler.bind(this));
this.ionPageElementRef.current.removeEventListener('ionViewDidLeave', this.ionViewDidLeaveHandler.bind(this));
this.ionPageElementRef.current.removeEventListener('ionViewWillEnter', this.ionViewWillEnterHandler);
this.ionPageElementRef.current.removeEventListener('ionViewDidEnter', this.ionViewDidEnterHandler);
this.ionPageElementRef.current.removeEventListener('ionViewWillLeave', this.ionViewWillLeaveHandler);
this.ionPageElementRef.current.removeEventListener('ionViewDidLeave', this.ionViewDidLeaveHandler);
}
}