Migrate to Svelte 5 + Vite 8 + TypeScript 5 (compatibility mode)#3386
Closed
jcfurey wants to merge 25 commits into
Closed
Migrate to Svelte 5 + Vite 8 + TypeScript 5 (compatibility mode)#3386jcfurey wants to merge 25 commits into
jcfurey wants to merge 25 commits into
Conversation
Security: - zip-slip guard in archive extraction (sanitize entry paths) - REMOTE control/API auth gate (require authenticated connection when a password is set) - harden WebRTC host window (deny navigation/new windows) - make export window contextIsolation explicit Correctness: - readExifData always resolves (fixes IPC hang) - save() queues concurrent saves instead of dropping them - requestMain registers listener before send; resolves when bridge absent - companion servers recreate listenable instances on restart; clear state on close - requestToMain uses a single shared dispatcher (no per-request global listener) - release thumbnail generation lock on timeout - single-pass now-playing token replacement - close fs.watch before re-watching downloads - per-process UUID fallback for machine id - order-independent deep compare in checkIfMatching - log previously-empty catches - normalize CRLF on CSV import Build/process: - add .nvmrc, engines field, and a PR CI workflow (build gate; format/svelte report-only) Verified: electron tsc type-check and production build pass.
- tmp -> ^0.2.7 (+ override) clears tmp path-traversal High across direct/grandiose/electron-builder - fast-xml-parser 5.4.1 -> ^5.8.0 (un-pinned) clears entity-expansion/builder-injection High - overrides shell-quote ^1.8.4 eliminates the critical svelte-inspector/open-in-editor/shell-quote chain - npm-run-all -> npm-run-all2@^9 (maintained drop-in) npm audit: 27 -> 21 (criticals 3 -> 0, highs 10 -> 7). Verified: electron tsc type-check and production build pass.
…or in audit docs - package-lock.json updated to match the dependency security fixes (tmp, fast-xml-parser, shell-quote override, npm-run-all2) - Correct an earlier audit error: package-lock.json IS committed/tracked; the .gitignore entry is a commented-out (inactive) line, not an active ignore rule. The 'no reproducible builds' finding is withdrawn. - ci.yml now installs with 'npm ci' (lockfile verified in sync via npm ci --dry-run)
- electron ^37.10.3 -> ^40.10.3; electron-builder ^26.15.2, electron-updater ^6.8.9 - better-sqlite3 -> ^12.10.0 (prebuilt binary available for Electron 40 ABI) - engines.node >= 22.12.0 (Electron 40 requirement); playwright CI node 20 -> 22 Stopped at 40 (not 42): Electron 41/42 ship V8 14 which removed v8::External::Value()/New(); better-sqlite3 12.10.0 fails to source-compile against it and has no prebuild for those ABIs. 40.10.3 is > 39.8.4 so it clears all flagged electron advisories. Verified: npm audit no longer flags electron (27->20 total, criticals 0, highs 7->6); electron tsc type-check passes; production build passes; app launches and fully renders under xvfb (screenshot confirmed). The single Playwright E2E test fails only on a stale welcome-popup selector, unrelated to Electron (and disabled in CI).
…on 40 The single E2E test had drifted from the UI and was failing on stale selectors: - pick the main window by URL (index.html) instead of firstWindow() (could be the splash) - drive the consolidated Initialize.svelte welcome popup (language dropdown, data-location folder picker, 'Get started!') instead of the old 3-step flow - guard the setup popup + onboarding guide so the test is idempotent (passes whether or not user data already exists) - group labels use data-title now -> select by text - save via Ctrl+S (the right-click 'Save' menu became 'Quick search') Verified passing under xvfb on both a fresh profile and an already-initialized one. Note: the test's FS_MOCK_STORE_PATH env var is not honored by the app, so state isn't isolated (harmless in fresh CI; noted as a follow-up).
…t RCE High) Pulls a patched serialize-javascript (7.0.5), clearing the High (GHSA-5c6j-r48x-rmvq) plus a @rollup/plugin-terser moderate. Zero-risk: the only consumer (config/building/rollup.config.mjs) isn't referenced by any build script (the active build uses Vite + tsc), and the terser() plugin API is unchanged (peer rollup ^4). npm audit: 20 -> 18 (criticals 0, highs 6 -> 5). Build verified.
…oderate) v8+ is ESM-only, but no dynamic-import conversion was needed: Electron 40 bundles Node >=22.12 which supports require() of ESM (music-metadata exposes a module-sync export condition and has no top-level await), so the existing static imports work. Only code change: v11 returns IPicture.data as Uint8Array instead of Buffer, so cover-art handling wraps it with Buffer.from(). Verified: electron tsc passes; svelte-check unchanged (ICommonTagsResult still exported); production build passes; E2E smoke test passes (decisive, since music-metadata is require()d at startup via responsesMain.ts). npm audit: 18 -> 16 (criticals 0, highs 5 -> 4).
…luded from PR diff and line counts
…ual diffs); keep linguist-generated
…ser (S4)
The variable-expression and number-input calculators used new Function("return "+x)(),
which executes arbitrary JS from user/network-reachable input. Replaced all four call
sites with evaluateExpression() — an arithmetic-only recursive-descent parser (numbers,
+ - * / % **, unary +/-, parentheses) that throws on any identifier/call/property access.
Verified: 19/19 unit checks (valid math computes correctly; alert(1), Math.PI, x=5,
__proto__, etc. all throw); production build passes.
…lint-plugin-svelte - Replace the three legacy eslintrc files with one flat config (config/linting/eslint.config.mjs). - eslint 8 (EOL) -> 9; @typescript-eslint 5 -> typescript-eslint 8; eslint-config-prettier 8 -> 10; eslint-plugin-jsdoc 39 -> 50; swap deprecated eslint-plugin-svelte3 -> eslint-plugin-svelte 3 (+ svelte-eslint-parser, supports Svelte 3); drop unused eslint-plugin-prefer-arrow. - Consolidate lint scripts into a single 'lint:eslint' (flat config scopes electron/frontend/svelte; --ext is removed in eslint 9). - Rule set kept behaviour-faithful: map removed v8 rules (ban-types -> no-restricted-types, no-empty-interface -> no-empty-object-type, no-var-requires -> no-require-imports, id-blacklist -> id-denylist), drop removed 'off' formatting rules, and turn off v8's newly-recommended promise/type rules to match the old config's intent. eslint:recommended intentionally not added (the old config never extended it). Verified: flat config loads and lints electron, frontend TS, and Svelte 3 components with no config/parsing errors; findings match the old rule set (pre-existing, mostly auto-fixable). Build passes; lockfile npm ci-verified. The old eslintrc files are kept (superseded; removable). The legacy config files (eslint.electron.json/eslint.frontend.json/eslint.svelte.js) are now unused.
eslint.electron.json / eslint.frontend.json / eslint.svelte.js are no longer referenced (replaced by config/linting/eslint.config.mjs in the ESLint 9 migration).
Mechanical autofixes across 291 files (prefer-const, consistent-type-imports, dot-notation, object-shorthand, no-var, etc.) from 'npm run lint', plus Prettier formatting on the touched files. Kept as a separate commit from the config migration. Verified: electron tsc passes; production build passes; svelte-check improved (199 -> 186 errors, no new ones); E2E smoke test passes. The remaining ~604 non-fixable findings (no-shadow / no-console / no-unused-vars debt) are left for follow-up.
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.
…stale build prereqs Adds a comprehensive build/test/packaging guide derived from the CI workflows, covering all three OSes (toolchains, Node 22.12, Python — incl. the macOS 3.11 node-gyp caveat, dev run, production build, the E2E test incl. the Linux xvfb wrapper, lint/format, and packaging/release + signing env vars). Also reconciles the README and CLAUDE.md: Linux needs libfontconfig1-dev + uuid-dev + libltc-dev (not just libfontconfig1-dev), notes Node 22.12+, and links to BUILDING.md.
…environment Pins the OS toolchain + Node 22.12 + Python + the native-build system libs (libfontconfig1-dev/uuid-dev/libltc-dev) + xvfb, installs deps (npm ci when a lockfile exists, else npm install) and runs the production build. Documented in BUILDING.md with usage (build/pack/test) and the lockfile traceability caveat. Note: targets Linux only (Electron builds per-platform). Not container-build-tested here (no Docker daemon in this env), but mirrors the verified CI/local build steps.
package-lock.json is already committed and in sync; switch build.yml, playwright.yml, and release.yml (Windows/Linux/macOS) from npm install to npm ci for reproducible, deterministic installs. Verified the lockfile records all platforms' optional native binaries (rollup/rolldown win32/darwin/linux with os/cpu constraints), so npm ci picks the right one per platform — and a clean npm ci (incl. native rebuild) passes on Linux. Also: add the missing uuid-dev/libltc-dev to playwright.yml's apt step (needed for the native rebuild), and replace the obsolete @rollup/rollup-x-gnu note in .gitignore (that npm bug was fixed in npm 10).
…oing forward Svelte 5 warns on self-closing non-void HTML elements (<div ... /> etc.). Converted all 204 occurrences across frontend + server components to explicit close tags (<div ...></div>) using eslint-plugin-svelte's autofixable svelte/html-self-closing rule (AST-safe), then prettier-formatted. Also enabled that rule in the project ESLint config (frontend) so it can't regress. Verified: full build passes (frontend + servers + electron); svelte-check warnings 299 -> 144 (the ~155 self-closing warnings cleared; 78 errors unchanged); E2E smoke test passes.
… labels, aria-selected)
- img alt="" on decorative output media / preview thumbnails / custom action icons (3)
- aria-label on icon-only inc/dec buttons in MaterialNumberInput + MaterialRadialPicker (4)
- aria-selected={false} on the non-selectable 'add new' dropdown option (2)
These are universally-correct improvements with no behaviour change. Build passes;
svelte-check warnings 144 -> 135; E2E passes.
The remaining a11y warnings are judgment-/risk-heavy and out of scope here (see PR notes):
~90 interactive <div> elements would need role+tabindex+keyboard handlers (changes
operator-UI tab order/key handling, which the build deliberately suppresses via onwarn),
plus intentional autofocus and structural floating-label cases.
The autofocus inputs/textareas are prop-driven (a caller opts in to focus a field when a dialog/field appears) — intentional operator UX. Marked with <!-- svelte-ignore a11y_autofocus --> rather than removing it. svelte-check warnings 135 -> 127.
The two .pickColor gradient swatches already had tabindex/aria-label but no keyboard
handler; added on:keydown={triggerClickOnEnterSpace} (reuses the existing click via the
shared helper, no new tab stops). svelte-check warnings 127 -> 125.
Inventories the remaining ~125 svelte-check a11y warnings (by category + component area), documents the fix patterns (using the existing clickable.ts helpers), the real risks (tab-order changes, key conflicts, manual-QA-only verification), a phased plan, and the policy decisions to make first (enforce a11y? remove the onwarn suppression? i18n aria-labels?).
jcfurey
pushed a commit
to jcfurey/freeshow
that referenced
this pull request
Jun 16, 2026
The committed public/index.html referenced the production bundle (./build/bundle.js), which only exists after `vite build` — a setProductionHTML() artifact that cleanBuilds.js never reverted before commit. Restore the dev source entry (/src/frontend/main.ts) to match upstream; postBuild.js still swaps in the bundle tags for production, cleanBuilds.js reverts. Fixes a broken `npm start` dev server. https://claude.ai/code/session_01GnucxHgJuqNxYHbeHonfRm
Contributor
Author
|
Re: the Svelte 5 transition "snap" (ref #1512) — flagging proactively, since it's the blocker cited when closing #3379. Root cause. Svelte made transitions local by default at the 3→4 boundary (carried into 5): a local transition only plays when its direct parent toggles — not when a block above it ({#key}/{#each}/{#if}) is created/destroyed. FreeShow's output transitions live inside {#key show} / {#each currentOut}, so post-migration they snap to the next frame. (svelte-migrate adds |global automatically; the manual compat-mode migration didn't — that's what regressed.) Fix. |global on the three output custom transitions — OutputTransition (text/media/PDF/effect output), the song-attribution text in Output.svelte, and ListView — plus a keying fix ({#key transitionId} instead of {#key show}) so the outgoing slide's text isn't orphaned during the global outro. svelte-check 0 / build clean. In this stack the fix lands in #3389 (the regression-fixes PR at the tail), to keep this one a pure framework bump — so transitions on this branch in isolation still snap; they animate once #3389 is in. Happy to pull it forward into this PR if you'd prefer it self-contained. Scope. This only restores the pre-Svelte-5 behavior — it deliberately does not touch the transition rework in #2169 (the identical-text flash / partial-fade that predates Svelte 5). FreeShow already has a custom transition system (utils/transitions.ts); this just makes the existing transitions play again rather than adding a new one. |
Contributor
Author
|
Build note — import.meta in the companion (iife) builds. A minor regression the Vite 4→8 bump exposes: src/server/remote/components/Auth.svelte resolves the login logo via new URL("…/freeshow.webp", import.meta.url). The companion apps build as iife (config/building/vite.config.servers.mjs → formats: ['iife']), and Vite 8 replaces import.meta with {} in iife output — so the URL's base is undefined and the remote login logo doesn't load. It's existing code (unchanged in this PR) and only the remote companion hits it; the main app and the other three companions are fine. I can fix it by referencing the logo via its served public path instead of import.meta.url (works in iife) and push it into this PR — or leave it to you if you'd rather set the companion-app asset strategy. Preference? |
Collaborator
|
Did you fix the Svelte transition issue? I would like to have only the related diff in a PR. |
jcfurey
pushed a commit
to jcfurey/freeshow
that referenced
this pull request
Jun 16, 2026
… no-op finding + ChurchApps#3391 pushback - reference/svelte5-transition-global: the isolated "related diff" vassbo asked for on ChurchApps#3386 (|global + transitionId keying, 4 files +12/-4), with the honest caveat that it's a no-op on the current Svelte-3 dev (transitions are global by default until the Svelte 4 local-default change), so it can't stand as a meaningful standalone PR — it belongs with the migration. - ChurchApps#3391: @vassbo declining the scripture-text protection ("not necessary"); drafted clarify-then-defer reply; feature stays on the fork. https://claude.ai/code/session_01GnucxHgJuqNxYHbeHonfRm
Contributor
Author
# output/transitions/OutputTransition.svelte
- <div class="transitioner" in:custom={inTransition || transition || {}} out:custom={outTransition || transition || {}} on:outrostart>
+ <div class="transitioner" in:custom|global={inTransition || transition || {}} out:custom|global={outTransition || transition || {}} on:outrostart>
# output/Output.svelte (attribution line, mirror branch unchanged)
- <p class="attributionString" transition:custom={transitions.text}>...
+ <p class="attributionString" transition:custom|global={transitions.text}>...
# slide/views/ListView.svelte
- <div class="center" transition:custom={transition}>
+ <div class="center" transition:custom|global={transition}>
# output/layers/SlideContent.svelte
let show = false
+ let transitionId = 0 // distinct identity per reveal so the |global outro doesn't orphan prev text
...
timeout = setTimeout(() => {
+ transitionId++ // bump before reveal
show = true
...
- {#key show}
+ {#key transitionId}
{#if show}Yes — fixed. Two parts: (1) add Do note that this only has an effect on Svelte 4/5. On the current Svelte-3 |
jcfurey
pushed a commit
to jcfurey/freeshow
that referenced
this pull request
Jun 16, 2026
…shback HTML-fetched all 8 upstream PR pages (2026-06-16): - CLOSED by vassbo: ChurchApps#3386 (Svelte 5 migration), ChurchApps#3387 (tests "not a benefit"), ChurchApps#3388 (strict types ":any churn"), ChurchApps#3389 (build/transition/regressions, silent). - OPEN with pushback: ChurchApps#3385 (safe-eval "fine to use new Function, local app"), ChurchApps#3390 ("what's the improvement? remove Rebuild button"), ChurchApps#3391 ("not necessary"). - OPEN no comment: ChurchApps#3384 (security/deps). Pattern: maintainer declining most modernization as unnecessary for a local desktop app. Closing ChurchApps#3386 makes the transition fix moot upstream for now. https://claude.ai/code/session_01GnucxHgJuqNxYHbeHonfRm
Collaborator
|
Okay, that makes sense. But I would still like it to not be stacked on another PR. Also have not checked if there's any other breaking changes. |
jcfurey
pushed a commit
to jcfurey/freeshow
that referenced
this pull request
Jun 16, 2026
…te5-vite8-migration) Per @vassbo's ChurchApps#3386 follow-up (un-stack the migration off dev). Branch built by cherry-picking the 11 migration commits onto dev, dropping ChurchApps#3384/ChurchApps#3385, reconciling deps, fixing Vite-8 type-only imports, and folding in the transition fix. npm run build passes; svelte-check 71 (compat mode). https://claude.ai/code/session_01GnucxHgJuqNxYHbeHonfRm
Contributor
Author
|
I'll get a new PR going for this sometime today. It'll likely be evening my time Central US. |
Collaborator
|
No need to rush it, but it's probably good to get the other major PRs done and merged first? |
jcfurey
pushed a commit
to jcfurey/freeshow
that referenced
this pull request
Jun 16, 2026
…#3385/ChurchApps#3391 closed, ChurchApps#3384 sole open Current upstream state (2026-06-16 ~18:35, via HTML fetch): - ChurchApps#3390 MERGED ("Nice, great!") — first modernization PR landed. - ChurchApps#3384 OPEN & ready, awaiting maintainer (sole live PR). - ChurchApps#3385 CLOSED (jcfurey conceded the safe-eval; ESLint-9 set aside). - ChurchApps#3391 CLOSED by vassbo (declined as unnecessary). - ChurchApps#3386 CLOSED/parked — standalone migration ready, opens after ChurchApps#3384. - ChurchApps#3387/ChurchApps#3388/ChurchApps#3389 closed. https://claude.ai/code/session_01GnucxHgJuqNxYHbeHonfRm
Contributor
Author
|
Sounds good. I'll be ready to rebase this and open up a fresh PR once you're happy with the state of #3384 |
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
3 participants
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.
🔗 Stacked on #3385 (ESLint 9 + safe-eval). Until it merges, the diff below also includes PR1+PR2's changes; GitHub collapses it once they land.
Upgrades the frontend stack to Svelte 5 / Vite 8 / TypeScript 5 in compatibility mode (no rune rewrite), preserving pre-migration type strictness so this PR stays a framework bump rather than a type overhaul (that's PR5).
Included:
The core Svelte 5 + Vite 8 + TS 5 migration
Close self-closing non-void HTML elements (Svelte 5 requirement) and enforce going forward
Zero-risk a11y fixes (alt text, icon-button labels, aria-selected) + acknowledge intentional autofocus; keyboard activation for focusable color swatches
ACCESSIBILITY.md (scope/roadmap for operator-UI keyboard a11y) and per-platform BUILDING.md
Dockerfile + .dockerignore for a reproducible Linux build; switch CI to npm ci
Verification: production build (npm run build) and the Playwright smoke test pass.
Follow-ups (kept out of this framework bump by design):
svelte-check reports ~76 migration type errors at this stage; they're resolved in the strict-mode PR (PR5). (svelte-check is not part of upstream CI.)
Two |global output-transition regressions from this migration are fixed in PR6.