Skip to content

fix(react): cleanup functions are execute for lifecycle hooks #28319

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
sean-perkins merged 6 commits into from
Oct 12, 2023
Merged

fix(react): cleanup functions are execute for lifecycle hooks #28319

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
5a3b63b
fix(react): lifecycle events are removed on page unmount
sean-perkins Oct 9, 2023
459b38c
Update packages/react/src/routing/OutletPageManager.tsx
sean-perkins Oct 11, 2023
d5782c8
chore: apply same comment to pagemanager
sean-perkins Oct 11, 2023
251b4d3
fix(react): cleanup functions are execute for lifecycle hooks
sean-perkins Oct 9, 2023
a66b174
chore: export interface
sean-perkins Oct 9, 2023
7eb58bd
chore: avoid unnecessary invoke of callback
sean-perkins Oct 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 101 additions & 5 deletions packages/react/src/contexts/IonLifeCycleContext.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ export interface IonLifeCycleContextInterface {
ionViewWillLeave: () => void;
onIonViewDidLeave: (callback: () => void) => void;
ionViewDidLeave: () => void;
cleanupIonViewWillEnter: (callback: () => void) => void;
cleanupIonViewDidEnter: (callback: () => void) => void;
cleanupIonViewWillLeave: (callback: () => void) => void;
cleanupIonViewDidLeave: (callback: () => void) => void;
}

export const IonLifeCycleContext = /*@__PURE__*/ React.createContext<IonLifeCycleContextInterface>({
Expand Down Expand Up @@ -36,19 +40,40 @@ export const IonLifeCycleContext = /*@__PURE__*/ React.createContext<IonLifeCycl
ionViewDidLeave: () => {
return;
},
cleanupIonViewWillEnter: () => {
return;
},
cleanupIonViewDidEnter: () => {
return;
},
cleanupIonViewWillLeave: () => {
return;
},
cleanupIonViewDidLeave: () => {
return;
},
});

export interface LifeCycleCallback {
(): void;
(): void | (() => void | undefined);
id?: number;
}

export interface LifeCycleDestructor {
id: number;
destructor: ReturnType<LifeCycleCallback>;
}

export const DefaultIonLifeCycleContext = class implements IonLifeCycleContextInterface {
ionViewWillEnterCallbacks: LifeCycleCallback[] = [];
ionViewDidEnterCallbacks: LifeCycleCallback[] = [];
ionViewWillLeaveCallbacks: LifeCycleCallback[] = [];
ionViewDidLeaveCallbacks: LifeCycleCallback[] = [];
componentCanBeDestroyedCallback?: () => void;
ionViewWillEnterDestructorCallbacks: LifeCycleDestructor[] = [];
ionViewDidEnterDestructorCallbacks: LifeCycleDestructor[] = [];
ionViewWillLeaveDestructorCallbacks: LifeCycleDestructor[] = [];
ionViewDidLeaveDestructorCallbacks: LifeCycleDestructor[] = [];

onIonViewWillEnter(callback: LifeCycleCallback) {
if (callback.id) {
Expand All @@ -63,8 +88,64 @@ export const DefaultIonLifeCycleContext = class implements IonLifeCycleContextIn
}
}

teardownCallback(callback: LifeCycleCallback, callbacks: any[]) {
// Find any destructors that have been registered for the callback
const matches = callbacks.filter((x) => x.id === callback.id);
if (matches.length !== 0) {
// Execute the destructor for each matching item
matches.forEach((match) => {
if (match && typeof match.destructor === 'function') {
match.destructor();
}
});
// Remove all matching items from the array
callbacks = callbacks.filter((x) => x.id !== callback.id);
}
}

/**
* Tears down the user-provided ionViewWillEnter lifecycle callback.
* This is the same behavior as React's useEffect hook. The callback
* is invoked when the component is unmounted.
*/
cleanupIonViewWillEnter(callback: LifeCycleCallback) {
this.teardownCallback(callback, this.ionViewWillEnterDestructorCallbacks);
}

/**
* Tears down the user-provided ionViewDidEnter lifecycle callback.
* This is the same behavior as React's useEffect hook. The callback
* is invoked when the component is unmounted.
*/
cleanupIonViewDidEnter(callback: LifeCycleCallback) {
this.teardownCallback(callback, this.ionViewDidEnterDestructorCallbacks);
}

/**
* Tears down the user-provided ionViewWillLeave lifecycle callback.
* This is the same behavior as React's useEffect hook. The callback
* is invoked when the component is unmounted.
*/
cleanupIonViewWillLeave(callback: LifeCycleCallback) {
this.teardownCallback(callback, this.ionViewWillLeaveDestructorCallbacks);
}

/**
* Tears down the user-provided ionViewDidLeave lifecycle callback.
* This is the same behavior as React's useEffect hook. The callback
* is invoked when the component is unmounted.
*/
cleanupIonViewDidLeave(callback: LifeCycleCallback) {
this.teardownCallback(callback, this.ionViewDidLeaveDestructorCallbacks);
}

ionViewWillEnter() {
this.ionViewWillEnterCallbacks.forEach((cb) => cb());
this.ionViewWillEnterCallbacks.forEach((cb) => {
const destructor = cb();
if (cb.id) {
Comment on lines +144 to +145

@thetaPC thetaPC Oct 12, 2023

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Wouldn't it be better to create the variable, destructor inside the if statement? That way there isn't an unused variable if cb.id doesn't exist.

This comment was marked as outdated.

Sign in to view

@sean-perkins sean-perkins Oct 12, 2023

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Following up on why I did this - took me a second to recall.

The callback may not have an id, but we still want to invoke the callback regardless. Otherwise the developers implementation within the lifecycle hook will never fire.

We only want to store the destructor callback if the callback has an id - which is assigned from the lifecycle hooks.

This was intentional.

this.ionViewWillEnterDestructorCallbacks.push({ id: cb.id, destructor });
}
});
}

onIonViewDidEnter(callback: LifeCycleCallback) {
Expand All @@ -81,7 +162,12 @@ export const DefaultIonLifeCycleContext = class implements IonLifeCycleContextIn
}

ionViewDidEnter() {
this.ionViewDidEnterCallbacks.forEach((cb) => cb());
this.ionViewDidEnterCallbacks.forEach((cb) => {
const destructor = cb();
if (cb.id) {
this.ionViewDidEnterDestructorCallbacks.push({ id: cb.id, destructor });
}
});
}

onIonViewWillLeave(callback: LifeCycleCallback) {
Expand All @@ -98,7 +184,12 @@ export const DefaultIonLifeCycleContext = class implements IonLifeCycleContextIn
}

ionViewWillLeave() {
this.ionViewWillLeaveCallbacks.forEach((cb) => cb());
this.ionViewWillLeaveCallbacks.forEach((cb) => {
const destructor = cb();
if (cb.id) {
this.ionViewWillLeaveDestructorCallbacks.push({ id: cb.id, destructor });
}
});
}

onIonViewDidLeave(callback: LifeCycleCallback) {
Expand All @@ -115,7 +206,12 @@ export const DefaultIonLifeCycleContext = class implements IonLifeCycleContextIn
}

ionViewDidLeave() {
this.ionViewDidLeaveCallbacks.forEach((cb) => cb());
this.ionViewDidLeaveCallbacks.forEach((cb) => {
const destructor = cb();
if (cb.id) {
this.ionViewDidLeaveDestructorCallbacks.push({ id: cb.id, destructor });
}
});
this.componentCanBeDestroyed();
}

Expand Down
12 changes: 12 additions & 0 deletions packages/react/src/lifecycle/hooks.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ export const useIonViewWillEnter = (callback: LifeCycleCallback, deps: any[] = [
useEffect(() => {
callback.id = id.current!;
context.onIonViewWillEnter(callback);
return () => {
context.cleanupIonViewWillEnter(callback);
};
}, deps);
};

Expand All @@ -20,6 +23,9 @@ export const useIonViewDidEnter = (callback: LifeCycleCallback, deps: any[] = []
useEffect(() => {
callback.id = id.current!;
context.onIonViewDidEnter(callback);
return () => {
context.cleanupIonViewDidEnter(callback);
};
}, deps);
};

Expand All @@ -30,6 +36,9 @@ export const useIonViewWillLeave = (callback: LifeCycleCallback, deps: any[] = [
useEffect(() => {
callback.id = id.current!;
context.onIonViewWillLeave(callback);
return () => {
context.cleanupIonViewWillLeave(callback);
};
}, deps);
};

Expand All @@ -40,5 +49,8 @@ export const useIonViewDidLeave = (callback: LifeCycleCallback, deps: any[] = []
useEffect(() => {
callback.id = id.current!;
context.onIonViewDidLeave(callback);
return () => {
context.cleanupIonViewDidLeave(callback);
};
}, deps);
};
27 changes: 19 additions & 8 deletions packages/react/src/routing/OutletPageManager.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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);
}
}

Expand Down
27 changes: 19 additions & 8 deletions packages/react/src/routing/PageManager.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -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);
}
}

Expand Down