Skip to content

Improve Ziggy setup in Inertia stacks #340

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

Merged
merged 6 commits into from
Dec 17, 2023

Conversation

bakerkretzmar
Copy link
Contributor

Ziggy maintainer here 👋🏻

This PR improves Ziggy's typings in the Inertia stacks and removes some unnecessary code.

Since v1.8.0 Ziggy ships its own types, so the @types/ziggy-js package is no longer needed. The types are included in the tightenco/ziggy Composer package; the changes to jsconfig.json and tsconfig.json allow them to be picked up from there.

The global Ziggy object being passed in .use(ZiggyVue, Ziggy) in the Vue stacks isn't necessary (it's made available globally by the @routes Blade directive) so I removed it. Without that, there are no references to that global object out of the box in any stack (and it's uncommon to reference that object directly), so I also removed the global type declaration for it.

The shared ziggy data in the HandleInertiaRequests middleware is only needed for SSR, so I removed it by default and added a step to the install command to insert it if that option is enabled. I also added a step there to include TypeScript type information corresponding to that shared data, so that page.props includes ziggy and it's typed correctly. This let me remove a couple of // @ts-expect-error comments too (but only for Vue, because the React stacks don't merge their own Inertia page prop type declarations into the global ones). The text replacements in this new method are pretty ugly I know, I'm happy to change that up however you think is best. I went down the road of adding alternate versions of those files first, but eventually came back this way because it would have been so much duplication.

I tested these changes by manually creating 8 new Laravel apps and setting up Breeze with every possible combination of Inertia React, Inertia Vue, JavaScript, TypeScript, SSR, and no SSR, and making sure they all scaffold the expected files and build successfully, which they do! Anecdotally, in VS Code, the intellisense around the route() function and other Ziggy stuff is much better now too.

This shouldn't be a breaking change because it will only affect new Breeze installations. Let me know if you have questions!

@taylorotwell taylorotwell merged commit 43af1cb into laravel:1.x Dec 17, 2023
@taylorotwell
Copy link
Member

Thanks!

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.

2 participants