Fix Inertia SSR errors #53
Conversation
|
Using SSR will also cause a flash of light theme for users who have previously selected a dark theme because the theme is saved in localstorage. I would love to see persisting theme in session or some other way, so we can inject the theme class in app blade during SSR and not have to wait for javascript to change it. |
|
Yeah, cookies can also work and I've gotten by using that route as well. Not sure if it necessarily falls on the responsibility of the starter kit though, as it's fairly easy to setup in user land if that's desired in the scaffolded app. |
|
For those stumbling upon this wondering about the dark mode flash, I have a branch demoing how to prevent it using this starter kit based on the changes from this PR. TL;DR, use a cookie to keep the dark mode state. |
| // For SSR, we can't access the window location, so can only render the layout on the client | ||
| if (typeof window === 'undefined') { | ||
| return null; | ||
| } |
There was a problem hiding this comment.
There is url in usePage hooks, why not just use that?
There was a problem hiding this comment.
We certainly could, both will accomplish the same thing I'm pretty sure. This is just a preference of being explicit about rendering on the server.
There was a problem hiding this comment.
I guess in that context I prefer that way too, thanks for the answer.
JoeyMckenzie
force-pushed
the
fix/inertia-ssr-errors
branch
from
90fcf59 to
011ee04
Compare
| "include": ["resources/js/**/*.ts", "resources/js/**/*.tsx"] | ||
| "include": [ | ||
| "resources/js/**/*.ts", | ||
| "resources/js/**/*.d.ts", |
There was a problem hiding this comment.
Would "resources/js/**/*.d.ts" not already be targeted with "resources/js/**/*.ts"?
There was a problem hiding this comment.
I don't think it is necessary, but I feel like it's not a bad thing to include because it helps with clarity. Some devs perfer to explicitly include the .d.ts 👍
There was a problem hiding this comment.
@RV7PR, correct. Technically not necessary, but I like to include them to be very explicit that the project include type declaration files. Similar to how the original Breeze templates worked as well.
composer.json
Outdated
| @@ -55,6 +55,11 @@ | |||
| "dev": [ | |||
| "Composer\\Config::disableProcessTimeout", | |||
| "npx concurrently -c \"#93c5fd,#c4b5fd,#fb7185,#fdba74\" \"php artisan serve\" \"php artisan queue:listen --tries=1\" \"php artisan pail --timeout=0\" \"npm run dev\" --names=server,queue,logs,vite" | |||
| ], | |||
| "dev:ssr": [ | |||
| "npm run build", | |||
There was a problem hiding this comment.
@JoeyMckenzie This should run npm run build:ssr instead of npm run build. Go ahead and get that updated and we'll merge this in.
Really appreciate the help!
There was a problem hiding this comment.
Good catch, updated.
|
@JoeyMckenzie Thanks for the PR. It's a good point what @Linux123123 brings up about the light mode flicker with SSR. I really like your progress with the Cookie solution. I think we'll wait to merge this in until we get this resolved. Also, I've created a PR for the Inertia SSR fixes to the Vue Starter Kit here, which I now see that you have left some comments in there as well. Thanks for that 😉 I was wondering if there is any advantage to use localstorage for non-ssr and cookies for SSR or if we should just use cookies for both. Let me know your thoughts. |
|
@tnylea Hacked on this a bit yesterday, coincidentally arriving at very similar to your last commit here (used Inertia middleware instead, both do the same thing). That fixes the SSR flashing, there's still the issue of CSR flashing though that should be fairly simple to just inline a Definitely should be considering using both local storage and cookies, since some folks can choose to disable cookies entirely. Relying on them solely would most likely break those user experiences, but it's definitely the solution for preventing the flash in SSR. Nice work on that last commit, I like it! |
|
@JoeyMckenzie, thanks! I'm sorry. We should have collaborated a little more if we had both been working on this solution. I agree. Using both cookies and local storage has its benefits. Who would have thought there would be so much involved with Light/Dark mode 🤣 But ultimately, this feels like a good solution. Let me know if you have any other feedback; otherwise, I'll sync up with the team and see if we can merge this soon. Thanks again! |
|
Thanks @JoeyMckenzie I cherry-picked those commits, works fine, almost in prod with those, hope this is merged ! |
|
@tnylea No worries! I'm just happy to be here and help where I can. 🤠 The only last bit I think we'd need is on that last commit by me, just adding some inline styles that match the shadcn styles set An inline style will prevent the flash, with the only nuisance being keeping the background colors in sync with whatever shadcn themes the user is running. Totally open to changing that too, more of just a quick 'n dirty solution to fix the CSR flash. Let me know. |
|
Thanks all. 👍 |
* fix: check for window when running ssr * chore: remove ssr files * fix: add ziggy to inertia middleware for ssr * chore: remove bootstrap ssr * chore: add script for ssr * chore: update composer script * Adding solution for Dark Mode flicker with SSR * Adding condition for system appearance and detecting via client-side immediately * feat: add inline style to prevent flash during CSR * formatting * package update --------- Co-authored-by: Tony Lea <[email protected]> Co-authored-by: Taylor Otwell <[email protected]>
There were a handful of errors when running SSR with Inertia, primarily around accessing the
windowand missing some middleware that's required for SSR contexts. This PR resolves those issues. I haven't checked the Vue starter kit yet, though Is suspect that same thing is probably happening there as well.