inertiajs / inertia

Inertia.js lets you quickly build modern single-page React, Vue and Svelte apps using classic server-side routing and controllers.
https://inertiajs.com
MIT License
6.55k stars 433 forks source link

CreateInertiaApp resolve runs twice at page first visit #1595

Open killjin opened 1 year ago

killjin commented 1 year ago

Version:

After we've setup our laravel + inertia app we found out that CreateInertiaApp() runs twice.

We tried to find a solution but haven't found one yet.

app.js

import { createApp, h } from "vue";
import { createInertiaApp } from "@inertiajs/inertia-vue3";
// or import { createInertiaApp } from "@inertiajs/vue3";
import DefaultLayout from "../../views/layouts/default";
import { createPinia } from "pinia";
import { importComponent } from "./import-component.js";
import { ZiggyVue } from "ziggy";
import { Ziggy } from "../../js/ziggy";
import { InertiaProgress } from "@inertiajs/progress";

InertiaProgress.init();
createInertiaApp({
  resolve: (name) => {
    console.log("123");
    const component = importComponent(name);

    // Set default layout if no layout specified on component
    component.default.layout ??= DefaultLayout;
    return component;
  },
  setup({ el, App, props, plugin }) {
    const pinia = createPinia();
    const app = createApp({ render: () => h(App, props) })
      .use(plugin)
      .use(pinia)
      .mixin({ methods: { route: window.route } })
      .use(ZiggyVue, Ziggy);

    // added ziggy route helper
    // shows laravel routes in vue
    app.config.globalProperties.$auth = props.initialPage.props.auth;
    app.mount(el);
  },
});

webpack.mix.js

mix
  .js("resources/assets/js/app.js", "public/js")
  .sass("resources/assets/sass/app.scss", "public/css")
  .webpackConfig(aliasesConfig)
  .sourceMaps()
  .vue({ version: 3 })
  .ziggy()
  .version();

image


I use laravel jetstream, also executed twice. app.js

import './bootstrap';
import '../css/app.css';

import { createApp, h } from 'vue';
import { createInertiaApp } from '@inertiajs/vue3';
import { resolvePageComponent } from 'laravel-vite-plugin/inertia-helpers';
import { ZiggyVue } from '../../vendor/tightenco/ziggy/dist/vue.m';

const appName = window.document.getElementsByTagName('title')[0]?.innerText || 'Laravel';

createInertiaApp({
    title: (title) => `${title} - ${appName}`,
    resolve: (name) => {
        console.log('resolve', name)
        return resolvePageComponent(`./Pages/${name}.vue`, import.meta.glob('./Pages/**/*.vue'))
    },
    setup({ el, App, props, plugin }) {
        return createApp({ render: () => h(App, props) })
            .use(plugin)
            .use(ZiggyVue, Ziggy)
            .mount(el);
    },
    progress: {
        color: '#4B5563',
    },
});

image

This results in resolving twice, and if resolve returns a different page the first time than the second, test that each page was resolved.

Why? Can you help us?

killjin commented 1 year ago

swapSomponent in router.serPage exec as a promise, then app render before reactive component change.

haltsir commented 1 year ago

I see the same behaviour with inertiajs/react:

"name": "@inertiajs/react", "version": "1.0.8",

import { createInertiaApp } from '@inertiajs/react'
import { createRoot } from 'react-dom/client'

createInertiaApp({
    resolve: async name => {
        console.log('Resolve', { name });
        return () => <div></div>;
    },
    setup({ el, App, props }) {
        console.log('CreateRoot', { props });
        createRoot(el).render(<App {...props} />)
    },
})

Resolve is logged twice.

reinink commented 1 year ago

Hey folks! You're right, it's possible for the resolve() callback to be called more than once — especially during the initial page load.

I'm not sure I see any practical issues with that — other than maybe a perceived performance problem, but that's certainly not something I've ever noticed.

The only other possible problem I could see here is if you're somehow using the resolve() callback for other things like page visit tracking or something. If that's the case I'd recommend not doing that and instead using the Inertia events instead: https://inertiajs.com/events

I'm not sure that I want to introduce a caching layer here unless completely necessary (although I appreciate your attempt here @craigrileyuk! 🙏).

Is there another reason why this is an issue that I'm not aware of?

dbushy727 commented 1 year ago

I recently faced an issue where the double render caused a flash in the UI, as state in my component differed between first and second renders.

caveat The double render actually pointed out a bug in my code 😳 that I was relying on the second render without noticing, so I should be thanking you instead of reporting an issue. I've fixed it in my own code, so I'm actually ok with the double render now. I'm not sure if it's worth fixing, but figured i'd post the issue I was facing in case it helps someone else, or is important enough to warrant fixing the double render.

The problem arises when you rely on the value of a React ref to make decisions in your component as their state is lagged by 1 render cycle. If you rely on the ref state, the second render can have a different value than the first render since the ref's value is getting hydrated on the first render when being passed a default value.

kinda contrived example:

function SomeComponent({ defaultSearch = '', records } : { defaultSearch: string; records: string[] }){
  const searchRef = useRef<HTMLInputElement>(null);
  const searchValue = searchRef.current?.value;

  function search(e: React.ChangeEvent<HTMLInputElement>) {
    // search
  }

  return (
      <div>
          <input ref={searchRef} defaultValue={defaultSearch} onChange={search} />
          {records.length === 0 &&
            (searchValue ? (
              <NoResults />
            ) : (
              <EmptyState />
            ))}
      </div>
  );
}

In this example above, when defaultSearch is some truthy string, the UI flashes from the EmptyState to NoResults real quick, because searchValue is undefined on first render, and then takes the value of defaultSearch on the second render.

I was able to fix it by updating the value of searchValue to fallback to defaultSearch, so not a big deal.

function SomeComponent({ defaultSearch, records } : { defaultSearch: string; records: string[] }){
  const searchRef = useRef<HTMLInputElement>(null);
  const searchValue = searchRef.current?.value ?? defaultSearch; // fix was adding defaultSearch here

  function search(e: React.ChangeEvent<HTMLInputElement>) {
    // search
  }

  return (
      <div>
          <input ref={searchRef} defaultValue={defaultSearch} onChange={search} />
          {records.length === 0 &&
            (searchValue ? (
              <NoResults />
            ) : (
              <EmptyState />
            ))}
      </div>
  );
}

There may be more useful cases for relying on ref being a step behind, and may cause issues for others. Not sure if thats worth pursuing, but figured it was worth mentioning as it took me a bit to figure out why I was seeing a flash.

hasan-ozbey commented 8 months ago

Same thing happens with fresh "Laravel 11 + Breeze + React" setup (via the Laravel installer). This isn't a vue-specific issue.

aihowes commented 6 months ago

Can confirm that this is not vue specific. I got the same double resolve on a Laravel 11 and React App (using Breeze).

I'm fairly new to React (coming from Vue) so I got a bit lost as to why I was getting refresh loop on a live filter index. (I had a useEffect watching for a data change, with a get())

After looking into React StrictMode, as I thought it was that, the double rendering did help me find a bug and use a ref to track change similarly mentioned by @dbushy727

wfern commented 1 month ago

Inertia resolves the component twice during initialization, once in createInertiaApp and again in router. I tried to reduce it to a single resolution and it works in my app, but I’m not sure about all the potential implications and I don't use SSR too.

I profiled it as well and didn’t see any performance improvement reducing to one. However, as someone mentioned before, the double initialization might cause some side effects in certain stacks?

haltsir commented 1 month ago

Not sure why the react label was removed on this ticket, the bug is still present.

@wfern how did you solve it for your app?

wfern commented 1 month ago

You can follow this gist to use the master version, update the code directly in node_modules, and create a patch if needed: gist link. Do it for the core and react package.

In my experiments, I only removed the resolution from the createInertiaApp function and set the initialComponent to null here: https://github.com/inertiajs/inertia/blob/11868a40bdeec670a52d7861abd0934ec5cc8dca/packages/react/src/createInertiaApp.ts#L85 This may cause issues if you’re using SSR.

It will be resolved again in the router: https://github.com/inertiajs/inertia/blob/11868a40bdeec670a52d7861abd0934ec5cc8dca/packages/core/src/router.ts#L478

Note: This was purely for curiosity. Removing this resolution may cause unwanted side effects. Since this change didn’t bring any performance improvements, I kept it as originally implemented.