Use vite flag for Paraglide isServer check #379
Conversation
✅ Deploy Preview for pauseai ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Wituareard
changed the title
|
@anthonybailey wdyt? Had to use a workaround to import the runtime outside Vite-processed code |
There was a problem hiding this comment.
Heh, we also both fixed this and the other endpoint TS error at the same time. I committed six minutes later, and didn't notice it merged because 585de1d had a tiny Tally warning fix too.
There was a problem hiding this comment.
This was the result of the broken Git configuration on one of my machines, I tried to merge from main
| typeof import('../../src/lib/paraglide/runtime.js') | ||
| > { | ||
| const runtimeString = await fs.readFile('src/lib/paraglide/runtime.js', 'utf-8') | ||
| const patchedRuntime = runtimeString.replace('import.meta.env.SSR', 'true') |
There was a problem hiding this comment.
So let me say this out loud? The scripts that call importRuntimeWithoutVite are clobbered so that if isServer was checked by the runtime in the course of its methods being called, it acts the same as it does when running as a server rather than running as a build.
(Presumably because SSR does not end up in the environment. I get it, and sorry.)
But... shouldn't it really be false? It gets used in a build context.
There was a problem hiding this comment.
Replied in the main thread
anthonybailey
left a comment
anthonybailey
left a comment
There was a problem hiding this comment.
As per individual comments, I get the ugly hack and need for it, but should it really be "true" we hardcode when the flag is missing?
|
Replacing with a static value is what Vite does as well and what enables tree-shaking. I guess the behavior for "runs during build" isn't defined because that isn't something Vite-processed code ever does. The docs say it indicates "runs on the server" because presumably it's meant to check for the existence of a server environment / the absence of a browser environment. But in the contexts where the file is actually imported it doesn't really matter I think so we can also set it to false. |
|
@anthonybailey Can I merge this tonight? |
|
Sure. The Boolean value was the only query I had, really. I advocate force pushing to l10-preview to see things work in non-en locales under true Netlify as well as the obvious checks of the PR preview in main's en-only mode. But if all was well under "netlify serve" that seems likely. I'll approve with that said. |
anthonybailey
left a comment
anthonybailey
left a comment
There was a problem hiding this comment.
As per thread. Fine given multiple locale testing.
|
Alright, finally tested it on the preview branch and everything seems to work |
|
Thanks! |
No description provided.