fix: script in app.html #13714
fix: script in app.html #13714
Conversation
martabal
force-pushed
the
fix/script-in-app.html
branch
from
dc5ba18
to
7e433d6
Compare
web/src/hooks.server.ts
Outdated
| const themePrepared = theme.replaceAll(/^export\s+/gm, ''); | ||
| const foucPrepared = fouc.replaceAll(/^import.*$/gm, themePrepared); | ||
| const scriptFouc = `<script>${transpileFile(foucPrepared)}</script>`; |
There was a problem hiding this comment.
Why do you have to strip out imports/exports? Why can this not be loaded as a module?
There was a problem hiding this comment.
All variables and methods should be placed within the script block. Correct me if I'm wrong, but if you include imports here, the browser will need to fetch those modules before executing the script, which will defeat the purpose.
There was a problem hiding this comment.
The solution isn't to manually resolve the dependencies by hand though. We should probably have the script be dependency-less and throw an error if there are any imports being used.
There was a problem hiding this comment.
So no shared modules? It's quiet nice to have theme.ts so we are sure that if one day we change one of the values here, there will be a lint/build failure
There was a problem hiding this comment.
I think there is value in getting linting, type-checking, etc. for this file. But you are getting that at the expense of manually managing module imports here, which I think isn't ideal. What if in the future theme.ts imports something from another file? Unless I'm mistaken that would just break this. What ends up happening is that you basically have to keep the dependency tree for this file in sync with this piece of code here. That's the job of a bundler, which you are in part replicating here. The two options I'd like to see instead would be either:
- This script has no dependencies, but other files can import from it. TLDR - move all the stuff that needs to be inlined, into this file and have everything else import from it.
- Define a custom entrypoint for vite which pulls everything in automatically into a single file and then inline that.
martabal
force-pushed
the
fix/script-in-app.html
branch
from
64fbf1f
to
e6f1853
Compare
| } | ||
|
|
||
| // should be the same values as the ones in constants.ts | ||
| enum Theme { |
There was a problem hiding this comment.
Does it not work to export these?
There was a problem hiding this comment.
Nope:
9:07:52 PM [vite] Error when evaluating SSR module /src/lib/utils/app.ts:
|- ReferenceError: localStorage is not defined
at /usr/src/app/src/lib/utils/app.ts:14:21There was a problem hiding this comment.
Can we tell svelte that this isn't a SSR module?
The only place where we have js is in
app.htmland that's causing multiple issues:Shared variables likecolorThemehave to be kept in sync across multiple files.That PR fixes all these issues it by moving that script to a complete new ts file
fouc.ts. That script is also simplified.During the first page load/the build, the import statement in
fouc.tsis directly replaced by the entire content of the imported file to not have to deal with imports. Then the ts is transpiled to js and that js replaces the %app.fouc% placeholder inapp.html.The script in
app.htmlis now: