Migrate to Svelte 5 + Vite 8 + TypeScript 5 (compatibility mode)#3396
Merged
vassbo merged 32 commits intoJun 17, 2026
Conversation
Collaborator
|
Check my reviews, and we probably don't need the new files. |
vassbo
requested changes
Jun 17, 2026
- Fixed certain space characters not rendered - Fixed deleting duplicated shows issue
jcfurey
pushed a commit
to jcfurey/freeshow
that referenced
this pull request
Jun 17, 2026
…d, all addressed Record the 2026-06-17 review on ChurchApps#3384 (18 inline comments / 14 files), all addressed on split/1-deps-and-security (c53fb58 -> 0c95118, +4 commits): reverted electron / music-metadata / fast-xml-parser to dev's pins and dropped the contested audit changes; kept the advisory fixes (terser, npm-run-all2, tmp + overrides). Updated the tracking table, summary table, and PR1 title/body; recorded the three deferred follow-up PRs and that the migration is now open as ChurchApps#3396. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GnucxHgJuqNxYHbeHonfRm
Toolchain:
- svelte 3 -> 5, vite 4 -> 8, @sveltejs/vite-plugin-svelte 2 -> 7, svelte-check 2 -> 4,
typescript 4.9 -> 5.9, @tsconfig/svelte 2 -> 5.
- Replace svelte-preprocess with vitePreprocess (svelte.config + both vite configs).
- Drop svelte-inspector fork + rollup-plugin-svelte (Svelte-3-only, used only by the
unused rollup.config.mjs).
Code changes (Svelte 5 / Vite 8 / TS 5 compatibility):
- Component instantiation: new App({target}) -> mount(App, {target}) (frontend + 5 server entries).
- Top.svelte: <script type="ts"> -> <script lang="ts"> (vitePreprocess needs lang).
- Connection.svelte: rename {@const connections} (Svelte 5 disallows shadowing a store name in a store subscription).
- Server type-only imports marked (Rolldown rejects type-as-value): Writable, ReceiverKey,
Deep*/Inferred/Nested, AutosizeTypes, StageItem/StageLayout.
- bonjour.ts: bonjour-service 1.4 exports Service as a value -> InstanceType<typeof Service> (TS 5).
- Raise the E2E test timeout (the flow is slightly slower under Svelte 5).
Status: full production build passes (frontend + servers + electron); the app launches and the
E2E user flow completes (verified via screenshot). KNOWN REMAINING: the electron process does not
exit cleanly after the test (worker-teardown hang) — needs investigation; svelte-check (v4) reports
~584 issues (mostly Svelte 5 deprecations/strictness) for a follow-up cleanup; full feature QA pending.
…umbrella) @tsconfig/svelte v5 turns on "strict" (incl. noImplicitAny), which the project intentionally kept off (it opts into only strictNullChecks/strictFunctionTypes/ noImplicitThis). Restoring strict:false drops svelte-check from 584 -> 78 errors (below the pre-migration ~186 baseline). Type-check only; the esbuild-based build is unaffected.
Re-point the migration at the current dev baseline instead of the ChurchApps#3384/ChurchApps#3385 stack: bump svelte 5 / vite 8 / typescript 5 / svelte-check 4 only, keeping dev's electron 37, vitest, version, and all other deps untouched (no security or ESLint-9 changes — those stay their own PRs). Drop eslint-plugin-svelte3 (Svelte-3-only, hard-incompatible with Svelte 5) and its lint:svelte step + config; .svelte files stay covered by svelte-check, prettier, and stylelint. A modern svelte ESLint setup belongs with the separate ESLint 9 modernization, not this framework bump. Regenerated package-lock.json. https://claude.ai/code/session_01GnucxHgJuqNxYHbeHonfRm
…ransitions Vite 8's bundler (rolldown) requires type-only imports to use `import type` (Svelte's isolatedModules). Dev code added since the original migration used value imports for types (Show, OutData, Item, ContextMenuItem, ProjectShowRef, DropAreas, …) — convert them (inline `type` where the module also exports values, e.g. svelte/store get + Unsubscriber, contextMenus). Also restore the output transitions broken by Svelte 4's local-by-default change: add |global to OutputTransition (in/out), the song-attribution text in Output.svelte, and ListView, plus a monotonic transitionId key in SlideContent so the outgoing slide's text isn't orphaned during the |global outro (the "snap" blocker, ref ChurchApps#1512). npm run build passes. svelte-check reports 71 deferred type errors (compat mode preserved — same backlog the original migration documented; not in CI). https://claude.ai/code/session_01GnucxHgJuqNxYHbeHonfRm
Vite 8 requires esbuild ^0.27 || ^0.28, but vitest@2.1.9 pulls vite@5 → esbuild@0.21.5, which npm hoists and then can't satisfy vite 8 — so `npm ci` (what upstream CI runs) failed to install even though `npm install` and the build tolerated it. vitest@4 peer-supports vite ^8, removing the second vite lineage. Regenerated a clean, ci-consistent lockfile. Verified: `npm ci` exit 0, `npm run build` 0 errors, unit tests pass on vitest 4, svelte-check 71 (compat mode, unchanged). https://claude.ai/code/session_01GnucxHgJuqNxYHbeHonfRm
…t.meta)
The remote companion built `freeshowLogo` with `new URL("…webp", import.meta.url)`.
Vite 8 (rolldown) replaces `import.meta` with {} in iife output (Rollup/Vite 4
polyfilled it), so the base compiled to the string "undefined" and
`new URL(dataURI, "undefined")` throws "Invalid URL" at runtime — breaking the
RemoteShow login screen, not just the logo. Import the asset instead; Vite
inlines the 378-byte webp as a data URI with no import.meta. Added a `*.webp`
ambient module declaration so svelte-check stays at 71.
Verified: 0 EMPTY_IMPORT_META from our code, build 0 errors, svelte-check 71, npm ci passes.
https://claude.ai/code/session_01GnucxHgJuqNxYHbeHonfRm
…ckfile, address review Reconstructed on current dev so the PR is just the framework migration: - regenerate package-lock.json against dev's lockfile; only the svelte/vite/ vitest/ts subtrees move, every unrelated dep stays at dev's pin - package.json: tmp ^0.2.7 (match dev's override) — was pulled to 0.2.5 during reconcile - config/building/vite.config.servers.mjs: drop the legacy a11y- check, keep only a11y_ Dropped per @vassbo's review (not re-applied during reconstruction): the ~200 self-closing conversions (kept />), BUILDING.md / ACCESSIBILITY.md / Dockerfile / .dockerignore, the a11y code pass, and the Node<26 pin/.nvmrc. eslint.svelte.js stays removed (Svelte-3 eslint-plugin-svelte3 blocks install on Svelte 5). The |global transition fix + SlideContent transitionId keying are preserved. npm ci, npm run build, and unit tests (37, vitest 4) all pass. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GnucxHgJuqNxYHbeHonfRm
dfa2503 to
a5f2efc
Compare
Match config/building/vite.config.servers.mjs (already cleaned per review): Svelte 5 only emits a11y_* warning codes, so the a11y- branch is dead. Keeps the two onwarn filters consistent. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GnucxHgJuqNxYHbeHonfRm
vassbo
requested changes
Jun 17, 2026
- main.ts: drop the redundant mount() comment (the call is self-evident)
- SlideContent.svelte: shorten the transitionId/{#key} comment to 2 lines
- vite.config.mjs: drop the dead `?? assetInfo.name` fallback — in Rollup 4
PreRenderedAsset.name is the @deprecated alias of names[0] (undefined when
names is empty), so `names?.[0] ?? ""` already covers every case
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GnucxHgJuqNxYHbeHonfRm
jcfurey
pushed a commit
to jcfurey/freeshow
that referenced
this pull request
Jun 17, 2026
… merged (2e6bbbd) - ChurchApps#3384 merged into upstream/dev; ChurchApps#3396 rebased on top, now sole open PR - Record the ChurchApps#3396 reconstruction (dropped contested new files / a11y / self-closing / node-pin) + 3 vassbo nit rounds resolved (tip f94218d) - Mark the import.meta iife-logo item fixed & review-verified Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GnucxHgJuqNxYHbeHonfRm
Per review — collapse the 3-line explanation to one line; the why (direct import → data URI, avoids import.meta.url blanked in iife) is unchanged. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GnucxHgJuqNxYHbeHonfRm
jcfurey
pushed a commit
to jcfurey/freeshow
that referenced
this pull request
Jun 17, 2026
…t trimmed (e8252fb) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GnucxHgJuqNxYHbeHonfRm
Collaborator
|
Let's hope this works fine. I will test a bit on a seperate branch first. |
jcfurey
pushed a commit
to jcfurey/freeshow
that referenced
this pull request
Jun 17, 2026
…svelte5 vassbo squash-merged the migration into a new upstream `svelte5` integration branch (not dev) to test first. Verified via upstream refs. Now 3 merged / 0 open; deferred follow-ups can proceed off upstream/dev. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01GnucxHgJuqNxYHbeHonfRm
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
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.
Standalone migration of the frontend + companion apps to Svelte 5 / Vite 8 / TypeScript 5, compatibility mode (no rune rewrite). Rebased onto current dev (now includes #3384). Pre-migration type strictness is preserved, so this stays a framework bump, not a type overhaul.
The reviewable surface is small — the 51-file count is the regenerated package-lock.json (GitHub collapses it) plus ~50 one-line changes:
Transition fix (the #1512 "snap"), folded in so the PR is self-contained:
Real fixes surfaced by the stricter toolchain:
Tooling: committed package-lock.json; vitest 2 → 4 (vitest 2 dragged in vite 5 / esbuild 0.21, blocking npm ci on vite 8); dropped eslint-plugin-svelte3 + its lint:svelte step (Svelte-3-only, blocks install on Svelte 5) — .svelte stays covered by svelte-check / prettier / stylelint.
Compatibility-mode caveat: svelte-check reports ~71 type errors at this stage (the @tsconfig/svelte v5 strict flags, deferred by design). The production build is unaffected, and svelte-check isn't in upstream CI.
Verification: npm ci ✓ · npm run build ✓ (frontend + companion servers + electron) · unit tests pass (vitest 4) · built app launches and renders.