Skip to content
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.

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

Make hasPathChanged on the Guard component configurable #39

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

glennverschooren
Copy link

Description

The parent route will rerender (destroy and initialize) each time the user navigates between child routes.
This can cause issues when fetching data inside a container/parent component or when the parent component must hold some state because the component will lose his current state each time the user navigates between the child routes.

Therefore I thought it will be a good idea to make the hasPathChanged function configurable so the developer can choose when the guards of the parent should be called.

Example:
Navigate to /users => check user guards
Navigate from /users to /users/123 => don't check the /user guards (don't destroy the /users route)
Navigate from /users/123 to /users/overview => don't check the /user guards (don't destroy the /users route)

What this does

It will allow the developer to set his own hasPathChanged function.

const isPathChanged = (
    routePrevProps: RouteComponentProps,
    routeProps: RouteComponentProps,
    path?: string | string[]): boolean =>  {
    // custome isPathChanged function
    return true;
}

<GuardedRoute
    path={route.path}
    pathChanged={isPathChanged}
    component={UsersComponent}/>

Copy link
Member

@beahuang beahuang left a comment

Choose a reason for hiding this comment

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

Thanks for contributing! Sorry for the late response, since this PR is adding a new Prop to the GuardedRoute can you update the docs to show that?

[routePrevProps, routeProps],
);
const hasPathChanged = useMemo(() => {
if (pathChanged && typeof pathChanged === 'function') {
Copy link
Member

Choose a reason for hiding this comment

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

It'd be more clear if we named this variable checkPathChanged so you know from a glance it should be a function

@@ -74,6 +74,11 @@ export interface BaseGuardProps {

export type PropsWithMeta<T> = T & {
meta?: Meta;
pathChanged?: (
Copy link
Member

Choose a reason for hiding this comment

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

Instead of adding the type here, we'll want to add it to the GuarderRoute and GuardProps below

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants