refactor: move the theme toggle into a file of its own - #219
Merged
Conversation
A script block in an .astro file is not typechecked; a .ts file is. The toggle handler and the prefers-color-scheme listener move out to src/scripts/theme-toggle.ts, and the block that held them becomes a third import alongside Bootstrap's two - a list of what runs on the page rather than a place where some of it happens to live. Nothing changes in the output. Astro was already bundling this block to its own file, so the emitted bundle is byte-identical, down to the content hash: Layout.astro_astro_type_script_index_0_lang.DHQd3bgp.js, 23047 bytes, before and after. astro check goes from 25 files to 26. The theme's other half stays where it is. It has to be is:inline in the head and run before the first paint, so it cannot be an import, and giving it a src would put a blocking request in front of the paint it exists to get ahead of. Inlining it from a file via ?raw does work - it would even save about 100 bytes a page on indentation - but it splits the reasoning about why that script is inline away from the script, and leaves a file that silently ships broken if anyone types TypeScript into it. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Moves the theme toggle out of the
<script>block inLayout.astroand intosrc/scripts/theme-toggle.ts.The motivation is typechecking: a script block in an
.astrofile is not checked, a.tsfile is.astro checkgoes from 25 files to 26 and passes clean. The block left behind becomes a third import alongside Bootstrap's two, which reads as a list of what runs on the page rather than as a place where some of it happens to live.Nothing changes in the output
Astro was already bundling this block into its own file, so this is a source move only. The emitted bundle is byte-identical, down to the content hash:
Layout.astro_astro_type_script_index_0_lang.DHQd3bgp.jsThe built HTML carries the same three script tags, and the inline head script is untouched.
Why the other half stays in the layout
The theme is picked by an
is:inlinescript in<head>that has to run before the first paint. That one cannot move:import, because a bundled module is deferred — which is the flash it exists to prevent.src, because that puts a blocking request in front of the paint it is trying to get ahead of.Inlining it from a file with
?rawandset:htmldoes work — I checked, and it would even save about 100 bytes a page, since the file's indentation is shallower than the.astronesting. It was not worth it: the comment above that script explains why it must be inline and pre-paint, and moving the code away from that reasoning makes the subtle part easier to break.?rawalso injects verbatim, so the file has to stay plain.jsforever or it ships broken TypeScript to the browser with nothing to catch it.🤖 Generated with Claude Code