Simplify usePage() hook in Vue 3 adapter

[reinink]

This PR simplifies the usePage() hook in the Vue 3 adapter. We use to run computed() on each value in the page object (in the root <App> component):

export function usePage() {
  return {
    props: computed(() => page.value.props),
    url: computed(() => page.value.url),
    component: computed(() => page.value.component),
    version: computed(() => page.value.version),
  }

In hindsight I honestly don't know why we did this. As far as I can tell you still always need to use usePage() in conjunction with a computed() call within your own components anyway, so this is redundant. Here's what you need to do today:

import { computed } from 'vue'

const appName = computed(() => usePage().props.value.appName)
                                          //  ^
                                          //  |- I hate that this .value is required

This would be worth it if you didn't need to still use computed() in your own components. But since you do, we might as well simplify usePage to this:

export function usePage() {
  return page.value
}

Doing that makes this hook easier to consume in your own components, because you no longer need to know that .value is required after each top level property:

import { computed } from 'vue'

const appName = computed(() => usePage().props.appName)
                                          //  ^
                                          //  |- No more .value required!

To me this is much better, but it's a breaking change. Obviously we're on the brink of an official 1.0 release, so now is the time to do it. It just means a slightly more difficult upgrade process for folks.

The upgrade process would just involve removing .value from their usePage() calls:

- const appName = computed(() => usePage().props.value.appName)
+ const appName = computed(() => usePage().props.appName)

Worth it? 🤔

[JoseLuisRNP]

@reinink I think this may have some problems, since we are returning only the value at the moment of executing the composable usePage, we lose the possibility of having computeds/watchers that react to changes in the data provided by usePage.

Anyway, this is just a warning since I don't know how the navigation between components works in Inertia and if the data is refreshed by reloading all the data and not mutating the ref this would not be a problem.

Here a simple demo (only recreating the problem, without Inertia) demo

Thanks for your work, Inertia is great!

[timvanuum]

In 99.9% of the cases this would be fine.
But I agree with @JoseLuisRNP, it feels like going against vue.

On the other hand, I think you can simply do:
watch(() => page, doSomeThing)

And the use case to watch page is extremely rare I think.

[reinink]

@JoseLuisRNP So from my testing, making the usePage() properties computed ahead of time doesn't actually have any benefit — you still always need to use computed() in your own components when consuming it.

I've tested this new implementation with Inertia's persistent layout in Vue 3 (see here), and it updated exactly as expected when navigating between pages. That was my main concern as well, but it seems to work perfectly.

That said, I am hoping to get another 1.0 beta out this week, so hopefully folks can battle test this change for me, just in case I missed anything.

[Tofandel]

@reinink

I've tested this new implementation with Inertia's persistent layout in Vue 3 (see here), and it updated exactly as expected when navigating between pages. That was my main concern as well, but it seems to work perfectly.

The problem with your test is that you're using the usePage mixin directly inside a computed which goes against the vue ideology, the mixins should always be called at the top level like this

Inertia v0 basic usage

const page = usePage()

const pageTitle = computed(() => page.props.value.title)
// This is reactive because we are dereferencing the computed's value within another computed, no problem here

Inertia v1

const page = usePage()

const pageTitle = computed(() => page.props.title)
// This is not reactive because the computed page has already been unrefed
const pageTitle = computed(() => usePage().props.title)
// This is reactive because the usePage method is unrefing a computed, basically doing page.value.props.title
// but it goes against vue's mixin best practices

Inertia v0 with vue`s best practices

const page = reactive(usePage().props) 

// Notice how we don't need to use `.value` thanks to `reactive`
const pageTitle = computed(() => props.title);
// This is still reactive because vue makes everything accessed within reactive another reactive proxy

// Even better make it your own composable
import {reactive} from 'vue';
import {usePage} from '@inertia/vue3'
export usePageProps = () => reactive(usePage().props)

// PROPS IS REACTIVE, WE DON'T EVEN NEED COMPUTED PROPERTIES!!! START THE REVOLUTION 🎊 
const props = usePageProps()

According to vue's guidelines, this PR solves a non existent problem and effectively creates reactivity problems for components that don't remount when swapping component

In hindsight I honestly don't know why we did this.

This is very probably why

But what you could do is make the page reactive instead of a ref since it's an object and instead of replacing it's value, you would need to assign it, this would effectively remove most reactivity issues without the need for .value

Or revert the change and wrap the return value in reactive

export function usePage() {
  return reactive({
    props: computed(() => page.value.props),
    url: computed(() => page.value.url),
    component: computed(() => page.value.component),
    version: computed(() => page.value.version),
  })
}

This removes the need from .value to access computed without breaking reactivity and without making it another major breaking change

[emargareten]

I had the same issue when I used const { props } = usePage()

[nielsquirynen]

Is there any chance this is getting reverted?
The previous usePage implementation seemed more in line with how composables are supposed to be implemented in Vue. Having to use the usePage hook inside a computed seems to go against the best practices.

[mathieutu]

I also had a lot of issues with that, so I've submitted a fix.

[Tofandel]

For reference on composable best practices https://vuejs.org/guide/reusability/composables.html#conventions-and-best-practices

I really think such a breaking change should have had a bit more thoughts before a release to v1, and reviews in hindsights. Given there where people already warning about this during the beta (and no 4 days of beta for a breaking change to go directly to release is not enough). No huge deal though it can be fixed without another breaking change by reverting and using reactive

Overall, there is a global disregard for feedback in inertia and most changes are unilateral and PR's and issues from outside the inertia team left unreviewed, which I think is a big problem with this project, some issues which are reported straight after a poorly tested release never even get a reply and most of them provide a PR which also goes unreviewed, and some other issues get immediate attention in a random manner. There was also recently a force push to master which left a bitter taste

@reinink How are those releases reviewed and tested, how are breaking changes decided (I really hope it's not just with the amount of thumbs up in a PR)? How are issues prioritized? Do you need more maintainers to help review issues and classify bugs in inertia? Do you have briefings or a process in place? Who has permission to do what? Because I'm not happy with the state of this project and I'd like to help, but I need to know if and how

[mathieutu]

For those who would like to fix this behaviour while waiting for #1469 to be merged, you can do:

import { Page, PageProps } from '@inertiajs/core'
import { usePage as buggyUsePage } from '@inertiajs/vue3'
import { computed, reactive } from 'vue'

/** @see https://github.com/inertiajs/inertia/pull/1469 */
const usePage = <SharedProps extends PageProps>(): Page<SharedProps> => (
  reactive({
    props: computed(() => buggyUsePage<SharedProps>().props),
    url: computed(() => buggyUsePage<SharedProps>().url),
    component: computed(() => buggyUsePage<SharedProps>().component),
    version: computed(() => buggyUsePage<SharedProps>().version),
    scrollRegions: computed(() => buggyUsePage<SharedProps>().scrollRegions),
    rememberedState: computed(() => buggyUsePage<SharedProps>().rememberedState),
    resolvedErrors: computed(() => buggyUsePage<SharedProps>().resolvedErrors),
  })
)

[Tofandel]

Indeed, that's exactly how it should have been implemented initially

It's also a good idea to make your own composable of libraries in general, to make your code more future proof against breaking changes, so that you only need to change one instance of the code to match your own spec, instead of the 1000 of page.props.value spread around your code like in this case

[nielsquirynen]

My current workaround:

import { usePage } from '@inertiajs/vue3';
import { toRefs } from '@vueuse/core';
import { InertiaPageProps } from '@/utils/types'; // Shared page props containing auth state etc

export const usePageProps = <
  Props extends Record<string, any>,
  Globals = InertiaPageProps,
>() => {
  return toRefs(computed(() => usePage<Globals & Props>().props));
};

// Page.vue
const { count } = usePageProps<{
   count: number;
}>()
 
watch(count, () => { ... })

[reinink]

Hey folks, these issues have been fixed (see #1469), and have been released in v1.0.4. Thanks for all your input/help! 🙏