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

[2.x] Fix Inertia SSR usage with Ziggy route function in setup() #1069

Merged
merged 3 commits into from
Jun 17, 2022

Conversation

prestonholt
Copy link
Contributor

This PR fixes an error where the Ziggy route() helper is undefined when route() is used in the setup() function and Inertia SSR is being used.

Steps to Reproduce

  1. Install Jetstream with Inertia SSR php artisan jetstream:install inertia --ssr
  2. In the Welcome.vue file, for example, use the route function inside <script setup>:
const test = route('login')
  1. Display the result in the template somewhere:
    {{ test }}
  2. Compile ssr.js npx mix --mix-config=webpack.ssr.mix.js
  3. Compile app.js npm run dev
  4. Start the SSR service node public/js/ssr.js
  5. Upon visiting the page, there will be an error in the node terminal (and the page will fall back to client-side rendering):
    ReferenceError: route is not defined

Proposed Solution

Using route() in the setup function will work using client-side rendering by default, because the @routes blade directive defines it globally. According to this answer, the route function should be injected when being used in a setup function. However, for the route function to be provided, the Ziggy Vue plugin must be used.

To do this in ssr.js, we would use ZiggyVue instead of the route mixin. This would keep route() available in all templates, and would make it available in setup functions if we inject it:

import { inject } from 'vue'
const route = inject('route')
const test = route('login')

This now works under SSR, but not when the page is client-side rendered: inject('route') will be undefined because we aren't using the ZiggyVue plugin in app.js.

Therefore, the route mixin also needs to be removed from app.js and replaced with:

import { ZiggyVue } from "ziggy";
//...
.use(ZiggyVue, Ziggy)

Ziggy refers to the definition const Ziggy by the @routes blade directive in <head>.

Breaking Changes

It doesn't seem like these changes will break anything in new Jetstream projects, whether using Inertia SSR or not. When only using client-side rendering, the route function can still be used in templates, setup, functions, etc. without being injected. The route function only needs to be injected when it's being used in the setup function and the page is being server-side rendered.

I wish there was a more elegant solution as I don't love having to const route = inject('route') where route is needed in script setup/computed functions/etc, so please let me know if there are any other ideas. I'm also not sure if this problem is big enough to warrant a change but wanted to put it out there in case others are having this issue

Ziggy Vue plugin allows `route()` to be injected into `setup()` function in components[1][2], which is needed for Inertia SSR since the @routes blade directive isn't available.

[1]: tighten/ziggy#518
[2]: tighten/ziggy#564 (comment)
@taylorotwell taylorotwell marked this pull request as draft June 15, 2022 07:09
@taylorotwell
Copy link
Member

Marking as draft until I can get a review from @jessarcher and @timacdonald - please mark as ready for review when that is done so it is surfaced back to me.

Copy link
Member

@timacdonald timacdonald left a comment

Choose a reason for hiding this comment

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

This looks good to me if we want to keep route globally available (not my preference, but I can see the utility). I've tested locally to confirm the issue and the fix for both SSR and non-SSR approaches.

Mixins are not supported by the composition API, and this seems like the Ziggy recommended approach.

Thanks @prestonholt

@timacdonald timacdonald marked this pull request as ready for review June 16, 2022 00:03
@taylorotwell taylorotwell merged commit d33e331 into laravel:2.x Jun 17, 2022
@prestonholt prestonholt deleted the inertia-ssr branch June 17, 2022 15:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants