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

[Possible Bug]: shouldRevalidate never fires #10774

Closed
chrisandrew-dev opened this issue Aug 10, 2023 · 3 comments
Closed

[Possible Bug]: shouldRevalidate never fires #10774

chrisandrew-dev opened this issue Aug 10, 2023 · 3 comments
Labels

Comments

@chrisandrew-dev
Copy link

What version of React Router are you using?

6.14.2

Steps to Reproduce

const routes = createRoutesFromElements(
    <Route
        path="/"
        element={<Layout />}
        loader={() => {
            console.log("Root route loader fired.")
            return ({ success: true })
        }}
        shouldRevalidate={() => {
            console.log("Root route shouldRevalidate callback fired.")
            return false
        }}
    >
        <Route
            path="/:slug"
            element={<Page />}
            loader={() => {
                console.log("Child route loader fired.")
                return ({ success: true })
            }}
            shouldRevalidate={() => {
                console.log("Child route shouldRevalidate callback fired.")
                return false
            }}
        />
    </Route>
)

const routeOptions = {
    future: {
        v7_normalizeFormMethod: true,
    },
}

const router = createBrowserRouter(routes, routeOptions)

const App = () => (
    <RouterProvider
        router={router}
    // fallbackElement={<LoadingPage />}
    />
)

export default App

Expected Behavior

I expected to see "Root route shouldRevalidate callback fired." in the console log upon returning to the "/" route. I also expected the "/" loader wouldn't fire.

I expected to see "Child route shouldRevalidate callback fired." in the console log upon returning to the "/:slug" route. I also expected the "/:slug" loader wouldn't fire.

Actual Behavior

loader callbacks on all routes fire on every navigation. shouldRevalidate callbacks never appear to fire.

@chrisandrew-dev chrisandrew-dev changed the title [Bug]: shouldRevalidate never fires [Possible Bug]: shouldRevalidate never fires Aug 10, 2023
@brophdawg11
Copy link
Contributor

Can you provide a reproduction? This behaves as expected in my reproduction: https://codesandbox.io/s/eloquent-mopsa-jqmmjv?file=/src/index.js

@chrisandrew-dev
Copy link
Author

chrisandrew-dev commented Aug 10, 2023

Screenshot 2023-08-11 at 9 21 46 am

The shouldRevalidate callback on the child route in your code pen doesn't appear to fire consistently either. Sometimes, after navigating back and forth several times, shouldRevalidate on the root route loader is skipped, and the loader callback fires again. Could you please clarify if this is the intended behaviour?

Could you please confirm if loaders are handled the same way when using the back/forward browser buttons or the Link component?

@brophdawg11
Copy link
Contributor

The shouldRevalidate callback on the child route in your code pen doesn't appear to fire consistently either.

When navigating from / -> /slug the child route is performing an initial load for that route instance, it's not a revalidation so shouldRevalidate doesn't run. Revalidation refers to the reloading of data that already exists in the UI, and is where you gain the ability to opt out. So when you go from /slug -> / - the root route loader data already exists, and shouldRevalidate is called to see if you wish to update it during that navigation.

Sometimes, after navigating back and forth several times, shouldRevalidate on the root route loader is skipped, and the loader callback fires again.

I'm not seeing this behavior on code sandbox or locally, and your console log above never shows the root loader re-executing. Can you provide a reproduction of this?

Could you please confirm if loaders are handled the same way when using the back/forward browser buttons or the Link component?

Yes, back/forward and Link navigations handle loaders the same.

I'm going to close this out since it seems everything is working correctly w.r.t. loaders and revalidations. Happy to re-open though if we can find a reliable reproduction of an issue.

@brophdawg11 brophdawg11 closed this as not planned Won't fix, can't repro, duplicate, stale Aug 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants