Skip to content

build: drop vitePreprocess from the shared Svelte config - #2228

Merged
chhoumann merged 1 commit into
masterfrom
build/drop-vite-preprocess
Oct 12, 2026
Merged

chhoumann merged 1 commit into
masterfrom
build/drop-vite-preprocess

Conversation

@chhoumann

@chhoumann chhoumann commented Oct 12, 2026 •

Copy link
Copy Markdown
Owner

Summary

svelte.config.mjs imported vitePreprocess, which loads Vite on every svelte-check run. The production build never reads this file and compiles every component without a preprocessor, so no component can depend on one. Without it, svelte-check and the Vitest Svelte plugin compile components the way the bundle does, which is what the file's comment asks for.

Measured on one GitHub runner, 8 rotating rounds per side (bench run 38288422505, master against this branch):

master this PR change
pnpm run check 13.86 s 13.52 s -0.34 s
pnpm run build-with-lint 21.32 s 20.88 s -0.44 s

On top of #2227's native check, the same change measured 4.02 → 3.65 s for check and 6.40 → 6.01 s for build-with-lint, with 10 rounds per side (bench run 38285760933). The saving is the same size on both, because both load the config the same way.

Changes

  • Remove the vitePreprocess import and the preprocess entry from svelte.config.mjs.

Testing / validation

  • pnpm install --frozen-lockfile, pnpm run build-with-lint, and pnpm run test pass. The suite ran 501 files: 7,019 tests passed and 28 were skipped.
  • main.js and styles.css are byte-identical to master. Their hashes are d83aa6eeaa77bbcfe0e3574f3ab1e67c57469506074c43f5e67087bf64ae6127 and 9467ef3622cd1137ce5a0148e16de23b9f5be87bc8c204fb6050c48f8f29994d.
  • The only <style lang> in the tree is lang="css", which Svelte handles without a preprocessor.

Independent of #2227. Either can merge first.

Release impact: none. The bundle is unchanged, and build: requests no release.

Checklist

Note

Remove vitePreprocess from shared Svelte config

Removes the vitePreprocess import and the preprocess property from svelte.config.mjs. Runes stay enabled through compilerOptions. Risk: tooling that loads this shared config no longer gets vitePreprocess(), so builds relying on it for CSS or TypeScript preprocessing may need their own setup.

Macroscope summarized 4645ed5.

Summary by CodeRabbit

  • Chores
    • Updated the app’s configuration. No user-visible changes are included in this update.

svelte-check and the Vitest Svelte plugin load svelte.config.mjs, and its
vitePreprocess() import pulls in Vite on every svelte-check run. The
production build never reads this file and compiles every component
without a preprocessor, so no component depends on one. Without it,
svelte-check and Vitest compile components the way the bundle does.
@coderabbitai

coderabbitai Bot commented Oct 12, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: 8578ac53-0a5c-4967-8fb7-219a6813c1ed

📥 Commits

Reviewing files that changed from the base of the PR and between d57eb15 and 4645ed5.


📒 Files selected for processing (1)
  • svelte.config.mjs

💤 Files with no reviewable changes (1)
  • svelte.config.mjs

Included review availability: This review used your included allowance. Your plan provides up to 8 included reviews per hour; 5 remain after this review.



📝 Walkthrough

Walkthrough

The Svelte config no longer imports vitePreprocess or sets the preprocess property. It retains compilerOptions: { runes: true }.

Changes

Svelte config

Layer / File(s) Summary
Remove preprocessing configuration
svelte.config.mjs
The config removes the vitePreprocess import and the preprocess property. It retains the runes compiler option.

Priority: ⬇️ Low

Estimated code review effort: 1 (Trivial) | ~3 minutes

Change: Refactor


Merge Risk: ⚪ Minimal · up to 4645e

No actionable risk from the config change was found. The PR is mergeable after normal checks.

Pre-merge checks | Passed 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly and concisely describes the main change: removing vitePreprocess from the shared Svelte configuration.
Docstring Coverage Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff. Analyzed 0 functions across 0…
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches
📝 Generate docstrings
  • Commit to this branch
  • Create a new PR

🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

A rabbit checked the config today,
And saw preprocessing hop away.
The runes setting stayed in place,
While imports cleared their little space.
Then off it bounced, pleased with the change!

Comment @coderabbitai help to get the list of available commands.

@chhoumann

Copy link
Copy Markdown
Owner Author

Ship verification: PASS+NOTES

Independent verification of this PR in fresh detached worktrees (head and parent), each with pnpm install --frozen-lockfile.

  • Head: 4645ed5 (build/drop-vite-preprocess)
  • Base: d57eb15 (master, also the head commit's only parent)
  • Patch-id: db126dd11553a4d55d125620bc28ee66b33fe61e (matches; the diff is the 3 deleted lines in svelte.config.mjs)

Checks

  1. Build: PASS. pnpm run build-with-lint exits 0 at head (svelte-check: 1509 files, 0 errors, 0 warnings). main.js and styles.css are byte-identical between parent and head (cmp clean). sha256 d83aa6eeaa77bbcfe0e3574f3ab1e67c57469506074c43f5e67087bf64ae6127 and 9467ef3622cd1137ce5a0148e16de23b9f5be87bc8c204fb6050c48f8f29994d, matching the PR description.
  2. Tests: PASS. pnpm run test at head: 501 files passed, 6 skipped; 7019 tests passed, 28 skipped; exit 0.
  3. Diagnostics parity: PASS. Injected the same three faults into both worktrees: <div onclick={() => {}}>X</div> appended to src/gui/svelte/DataUnreadable.svelte, an unused .qa-unused-rule-zz rule inside the <style> of src/gui/components/Toggle.svelte, and export const qaTypeErr: number = "str" in src/quickAddSettingsTab.ts. pnpm run check -- --output machine produced an identical normalized set in both trees: css_unused_selector at Toggle.svelte 62:2, a11y_click_events_have_key_events and a11y_no_static_element_interactions at DataUnreadable.svelte 74:1, and the TS2322 error at quickAddSettingsTab.ts 502:14; COMPLETED 1509 FILES 1 ERRORS 3 WARNINGS in both. Edits were reverted afterwards. Separately, a scratch script outside the repo ran svelte/compiler 5.57.2 directly on the pristine blobs of all 28 style-bearing components (WaitCommand.svelte with <style lang="css"> first), with and without vitePreprocess() from @sveltejs/vite-plugin-svelte 7.3.1: the preprocessed source equals the original for every component, and compiled CSS, client JS (css external and injected), and compiler warnings are identical for all 28.
  4. TypeScript-only syntax: PASS. All 57 components use <script lang="ts">; 27 <style> and 1 <style lang="css">. A scan of every script block for enums, namespaces, declare blocks, decorators, constructor parameter properties, import-equals, export-assignment, and triple-slash references found zero uses (the one regex hit was the word "accessor" in a comment). Plain compile() without a preprocessor throws typescript_invalid_feature for enum, namespace, decorator, and parameter property test inputs, so the production esbuild-svelte path would already have failed if any component used them.
  5. CI and bench: PASS with notes. Every check run on 4645ed5 is success (Test, Build + Lint, Docs, CodeQL x3, Dependency Review, PR title, Macroscope), plus CodeRabbit and Devin statuses success; Platform Test is skipped by matrix and the Cursor automation is neutral, as on other PRs. Bench run 38288422505 (CANDIDATES master nopre-master final final3, 8 rotating rounds) prints master check 13.86 s vs nopre-master 13.52 s and build-with-lint 21.32 s vs 20.88 s; run 38285760933 (final vs final-nopre, 10 rounds) prints check 4.02 s vs 3.65 s and build-with-lint 6.40 s vs 6.01 s. Both tables match the PR body exactly, all exits 0.

Notes

  • The bench candidate branches (bench/nc-nopre-master, bench/nc-final, bench/nc-final-nopre, bench/nc-final3) are no longer on origin, so I could not re-inspect what each candidate contained; the run.sh in the bench commits shows they were separate branches fetched at run time. The timing numbers are taken from the logs as published. A local alternating timing sample was inconclusive because this machine was at load average 32 to 43 on 12 cores, with multi-second variance per round.
  • In vite-plugin-svelte 7.3.1, vitePreprocess() with no options installs only a style handler (script preprocessing is opt-in), so TypeScript in script blocks was already handled natively by Svelte on master. The only behavior this PR removes is Vite's CSS pass over the 28 style blocks, which the scratch comparison shows is a byte-level no-op on this tree.
  • @sveltejs/vite-plugin-svelte stays as a devDependency because vitest.config.mts still uses its svelte() plugin, so no dependency change is needed.

@chhoumann
chhoumann merged commit ca6ea85 into master Oct 12, 2026
16 checks passed
@chhoumann
chhoumann deleted the build/drop-vite-preprocess branch October 14, 2026 06:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant