Conversation
There was a problem hiding this comment.
Pull request overview
Updates the project’s Svelte integration to Svelte 5, including SSR/pre-render adjustments and replacing svelte-navigator with a small in-repo router for the Contribute flow.
Changes:
- Bump dependencies to Svelte 5 and
@urql/svelte4.x, removingsvelte-navigator. - Update webpack SSR/pre-render pipeline to use
svelte/server’srender()and Svelte 5 compiler options. - Introduce a minimal
Router/Route/Linkimplementation and refactor Contribute pages to use it + context-based route state.
Reviewed changes
Copilot reviewed 16 out of 17 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| webpack/svelte-pre-render-plugin.js | Switch SSR rendering to svelte/server render() |
| webpack.pre-render.js | Update Svelte loader compiler target to server + add svelte/* externals |
| webpack.common.js | Adjust Svelte resolution conditions and compiler options |
| svelte/lib/router.js | New entrypoint exporting in-repo router components |
| svelte/lib/Router.svelte | New Router context provider for basepath + current path |
| svelte/lib/Route.svelte | New route matcher that sets route context |
| svelte/lib/Link.svelte | New Link component that resolves relative paths against basepath |
| svelte/contribute/index.js | Update client bootstrapping to Svelte 5 hydrate() API |
| svelte/contribute/Tile.svelte | Swap svelte-navigator Link for in-repo Link |
| svelte/contribute/Steps.svelte | Switch from let:location to context-based route location |
| svelte/contribute/Picker.svelte | Switch from location prop to context-based route location |
| svelte/contribute/Header.svelte | Swap svelte-navigator Link for in-repo Link |
| svelte/contribute/Contribute.svelte | Replace routing + update @urql/svelte client setup for v4 |
| svelte/contribute/Area.svelte | Switch from location prop to context-based route location |
| package.json | Bump Svelte + urql deps and remove svelte-navigator |
| package-lock.json | Lockfile updates for Svelte 5 / urql 4 dependency graph |
| eslint.config.mjs | Adjust import/extensions rule for .svelte imports in JS/TS |
Comments suppressed due to low confidence (1)
package.json:104
- After bumping to Svelte 5, the current devDependencies still include
svelte-loader@3.2.4andsvelte-preprocess@5.0.x/5.1.x, which (per their peerDependencies in the lockfile) only declare compatibility with Svelte 5 pre-release ranges (e.g.^5.0.0-next.*) rather than stable^5.0.0. This can causenpm install/CI to fail due to peer-dependency conflicts. Upgrade these tooling packages to versions whose peer ranges include stable Svelte 5, or adjust the Svelte version accordingly.
"svelte": "^5.0.0",
"tom-select": "^2.5.2",
"underscore": "^1.13.8"
},
"devDependencies": {
"@babel/core": "^7.29.0",
"@babel/plugin-transform-runtime": "^7.29.0",
"@babel/preset-env": "^7.29.0",
"@stylistic/stylelint-plugin": "^4.0.1",
"@mozilla-protocol/core": "22.0.0",
"@types/underscore": "^1.13.0",
"autoprefixer": "^10.4.27",
"babel-loader": "^9.2.1",
"baseline-browser-mapping": "^2.10.0",
"browser-sync": "^3.0.4",
"chai": "^4.5.0",
"chai-lint": "0.1.1",
"color-string": "^1.9.1",
"compression-webpack-plugin": "^10.0.0",
"concurrently": "^8.2.2",
"copy-webpack-plugin": "^11.0.0",
"css-loader": "^7.1.4",
"cssnano": "^7.1.3",
"env-cmd": "^10.1.0",
"eslint": "^9.31.0",
"eslint-import-resolver-webpack": "^0.13.10",
"eslint-plugin-import": "^2.29.1",
"exports-loader": "^5.0.0",
"glob": "^8.1.0",
"html-webpack-plugin": "^5.6.6",
"image-minimizer-webpack-plugin": "^3.8.3",
"imagemin": "^9.0.1",
"imagemin-svgo": "^10.0.1",
"jsdom": "^21.1.2",
"kss": "^3.1.0",
"locutus": "^2.0.39",
"mini-css-extract-plugin": "^2.10.1",
"mocha": "^11.7.5",
"onchange": "^7.1.0",
"path-parse": "^1.0.7",
"postcss": "^8.4.31",
"postcss-custom-properties": "^13.3.12",
"postcss-loader": "^8.2.1",
"sass": "^1.98.0",
"sass-loader": "^16.0.7",
"sharp": "^0.34.5",
"sinon": "^21.0.2",
"sinon-chai": "^3.7.0",
"source-map-support": "^0.5.21",
"style-loader": "^3.3.1",
"stylelint": "^16.26.1",
"stylelint-config-recommended-scss": "^16.0.2",
"stylelint-order": "^7.0.1",
"stylelint-scss": "^6.14.0",
"svelte-loader": "^3.2.4",
"svelte-preprocess": "^5.0.1",
"webpack": "^5.105.4",
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
webpack/svelte-pre-render-plugin.js
Outdated
| const { render } = rerequire("svelte/server"); | ||
| const Component = rerequire(targetPath).default; |
escattone
left a comment
There was a problem hiding this comment.
Claude identified the following:
Bug: Landing page Picker heading regression
svelte/contribute/Picker.svelte:10-19 — Picker now gets location from the "route" context instead of as a prop. On the landing page (Route path="/"), the Route component sets location = { pathname: "/" }, which is truthy. Previously, Landing.svelte rendered <Picker /> without passing a location prop, so location was undefined.
This means the landing page will now show "Other ways to contribute" instead of "Pick a way to contribute". To fix this, Route.svelte could set location = null for the root path, or Picker could check for a more specific condition (e.g., location?.pathname !== "/").
Dead props passed in Area.svelte
svelte/contribute/Area.svelte:25-26 — Area still passes {location} as a prop to both <Steps> and <Picker>:
<Steps {location} {...steps} />
<Picker {location} />
But neither Steps nor Picker declare export let location anymore — they both get it from context. These prop passes are dead code and should be removed. In Svelte 5, unknown props will produce a dev-mode warning.
Minor: svelte:component is deprecated in Svelte 5
svelte/contribute/Steps.svelte:48 — <svelte:component this={step} {...rest} /> still works in Svelte 5 but is deprecated. In Svelte 5 you can use <step {...rest} /> directly (dynamic component via capitalized variable). Low priority since it's backward compatible.
35a3103 to
7785dc3
Compare
|
@escattone PR updated |
No description provided.