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.

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

Jump to bottom

Data hydratation issue with partial reload on initial page visit #1890

Open
Gregory-Gerard opened this issue Jun 9, 2024 · 2 comments
Open

Data hydratation issue with partial reload on initial page visit #1890

Gregory-Gerard opened this issue Jun 9, 2024 · 2 comments
Labels
investigate The issue needs further investigating react Related to the react adapter

Comments

@Gregory-Gerard
Copy link

Gregory-Gerard commented Jun 9, 2024 •
edited
Loading

Version:

  • @inertiajs/react version: 1.1.0

Describe the problem:

Implementing lazy loading with Inertia::lazy in Laravel works well for asynchronous data fetching and improves Time To First Byte (TTFB). Using router.visit navigates the application effectively. However, an issue arises when reloading the page: the data fetch initiates correctly but the data itself fails to update in the UI.

Steps to reproduce:

When calling router.reload in a useEffect, we notice that the data is not updated correctly when reloading page using the browser.

// use-partial-reload.ts
import { useEffect } from 'react';
import { router } from '@inertiajs/react';

export default function usePartialReload(only: string[]) {
  useEffect(() => {
    router.reload({
      only,
    });
  }, []);
}

// component.tsx
import React from 'react';
import { Head, usePage } from '@inertiajs/react';
import { usePartialReload } from '@/hooks';

export default function Index() {
  usePartialReload(['contacts']);
  // contacts stay undefined when reloading the page, but get hydrated when using `router.visit`
  const { contacts } = usePage().props;

  return (
    <>
      <Head title="Contacts" />
      {contacts?.data.length}
    </>
  );
}

Unfortunately, this issue has already been raised, for example: #1547

The setTimeout(..., 0) solution still works but something seems off about it.

Debug:

I've tried to do a little debugging and apply some logs to understand what's going on. When the Inertia application is created for the first time, the router.init method is launched, followed by router.handleInitialPageVisit. We can then notice that the setPage method is called, in which the element of interest is this one:

inertia/packages/core/src/router.ts

Line 469 in 683155c

if (visitId === this.visitId) {

So I started logging this condition, and indeed, on the first load the visitId changes because the router.reload is called before the router.handleInitialPageVisit. So when we receive the router.reload result, because of this condition, the result is ignored because the visitId has been updated by router.handleInitialPageVisit.

image

A possible solution?

I've never contributed to Inertia, so certainly this solution may have edge cases, but having a bit of fun with the sources I noticed that setPage can take a visitId as a parameter to avoid regenerating it.

So, if I detect that a visitId has already been initialized before the handleInitialPageVisit, I can simply reuse it to avoid regenerating it and ignore the router.reload result.

diff --git a/packages/core/src/router.ts b/packages/core/src/router.ts
--- packages/core/src/router.ts
+++ packages/core/src/router.ts
@@ -83,9 +83,9 @@
   }
 
   protected handleInitialPageVisit(page: Page): void {
     this.page.url += window.location.hash
-    this.setPage(page, { preserveState: true }).then(() => fireNavigateEvent(page))
+    this.setPage(page, { preserveState: true, visitId: this.visitId ?? undefined }).then(() => fireNavigateEvent(page))
   }
 
   protected setupEventListeners(): void {
     window.addEventListener('popstate', this.handlePopstateEvent.bind(this))

View source:

inertia/packages/core/src/router.ts

Line 87 in 683155c

this.setPage(page, { preserveState: true }).then(() => fireNavigateEvent(page))

When using this patch, here is the result:
CleanShot 2024-06-09 at 19 19 14
And the UI updates well, whether following a router.visit or a browser reload.

If this solution is suitable, I can make a PR with test cases if necessary. In the meantime, I'm keeping the setTimeout(..., 0) which works for the moment. Also, many thanks for this library.
@Gregory-Gerard Gregory-Gerard added the react Related to the react adapter label Jun 9, 2024
@liliangiraudo5
Copy link

I have the same problem !

@driesvints driesvints added the investigate The issue needs further investigating label Jun 18, 2024
@payalord
Copy link

payalord commented Jul 1, 2024

Looks like I have the same problem, but with router.get and router.visit in useEffect too. In my case I needed to redirect user to /login page from / initially if user is not logged in. So I used `router.get('/login') with useEffect. Probably better to move this logic to backend instead. But still, I were expect router to work in useEffect even initially.

@girardinsamuel girardinsamuel mentioned this issue Oct 4, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
investigate The issue needs further investigating react Related to the react adapter
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@driesvints @payalord @Gregory-Gerard @liliangiraudo5