-
-
Notifications
You must be signed in to change notification settings - Fork 58
Upgrade Svelte to v5. Replace Jest with Vitest. Upgrade other packages #460
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
304cda3
bb6333b
9d67a36
f9d2fcd
faae69b
99bd7d6
2356c3e
798dc80
347b129
05383f3
4e20c7a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
This file was deleted.
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -2,22 +2,25 @@ import svelte from "rollup-plugin-svelte"; | |
| import resolve from "@rollup/plugin-node-resolve"; | ||
| import commonjs from "@rollup/plugin-commonjs"; | ||
| import livereload from "rollup-plugin-livereload"; | ||
| import { terser } from "rollup-plugin-terser"; | ||
| import { minify } from "rollup-plugin-esbuild-minify"; | ||
| import { spawn } from "child_process"; | ||
|
|
||
| const production = !process.env.ROLLUP_WATCH; | ||
|
|
||
| export default { | ||
| input: "src/<%= orgName %>-<%= projectName %>.js", | ||
| output: { | ||
| sourcemap: true, | ||
| format: "system", | ||
| format: "esm", | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a breaking change, and should be reflected in changesets. |
||
| name: null, // ensure anonymous System.register | ||
| file: "dist/<%= orgName %>-<%= projectName %>.js", | ||
| }, | ||
| plugins: [ | ||
| svelte({ | ||
| // enable run-time checks when not in production | ||
| dev: !production, | ||
| compilerOptions: { | ||
| // enable run-time checks when not in production | ||
| dev: !production, | ||
| }, | ||
|
|
||
| emitCss: false, | ||
| }), | ||
|
|
@@ -43,7 +46,7 @@ export default { | |
|
|
||
| // If we're building for production (npm run build | ||
| // instead of npm run dev), minify | ||
| production && terser(), | ||
| production && minify(), | ||
| ], | ||
| watch: { | ||
| clearScreen: false, | ||
|
|
@@ -58,7 +61,7 @@ function serve() { | |
| if (!started) { | ||
| started = true; | ||
|
|
||
| require("child_process").spawn("npm", ["run", "serve", "--", "--dev"], { | ||
| spawn("npm", ["run", "serve", "--", "--dev"], { | ||
| stdio: ["ignore", "inherit", "inherit"], | ||
| shell: true, | ||
| }); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| import "@testing-library/jest-dom/vitest"; | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Pretty weird that |
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| <script> | ||
| export let name; | ||
| <script lang="ts"> | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For create-single-spa's react implementation, typescript is an option passed to the generator, with EJS template for adding typescript syntax. We should add the same for svelte. |
||
| let { name } = $props(); | ||
| </script> | ||
|
|
||
| <style> | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -1,33 +1,33 @@ | ||||||
| { | ||||||
| "name": "<%= name %>", | ||||||
| "type": "module", | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is a breaking change and should be described in a changeset. |
||||||
| "scripts": { | ||||||
| "build": "concurrently <%- packageManager %>:build:*", | ||||||
| "build:rollup": "rollup -c", | ||||||
| "start": "rollup -c -w", | ||||||
| "serve": "sirv dist -c", | ||||||
| "test": "jest", | ||||||
| "test": "vitest", | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did the jest configuration no longer work after the upgrade to svelte? Why switch from jest to vitest?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To simplify the test setup. Jest requires some hurdles to go through with ESM.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm ok switching to vitest for this generator. Jest typescript esm with node flags for vm modules is a pain. |
||||||
| "prepare": "husky install", | ||||||
| "format": "prettier --write --plugin-search-dir=. .", | ||||||
| "check-format": "prettier --plugin-search-dir=. --check ." | ||||||
| }, | ||||||
| "devDependencies": { | ||||||
| "@babel/core": "^7.23.3", | ||||||
| "@babel/preset-env": "^7.23.3", | ||||||
| "@rollup/plugin-commonjs": "^20.0.0", | ||||||
| "@rollup/plugin-node-resolve": "^13.0.4", | ||||||
| "@testing-library/jest-dom": "^5.17.0", | ||||||
| "@testing-library/svelte": "^3.0.3", | ||||||
| "babel-jest": "^27.5.1", | ||||||
| "concurrently": "^6.2.1", | ||||||
| "jest": "^27.5.1", | ||||||
| "prettier": "^2.3.2", | ||||||
| "prettier-plugin-svelte": "^2.3.1", | ||||||
| "rollup": "^2.56.3", | ||||||
| "@babel/core": "^7.27.7", | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The babel.config.js was deleted - was it only being used by jest? If so, are
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I forgot to remove them. Will do so. |
||||||
| "@babel/preset-env": "^7.27.2", | ||||||
| "@rollup/plugin-commonjs": "^28.0.6", | ||||||
| "@rollup/plugin-node-resolve": "^16.0.1", | ||||||
| "@sveltejs/vite-plugin-svelte": "^5.1.0", | ||||||
| "@testing-library/jest-dom": "^6.6.3", | ||||||
| "@testing-library/svelte": "^5.2.8", | ||||||
| "concurrently": "^9.2.0", | ||||||
| "prettier": "^3.6.2", | ||||||
| "prettier-plugin-svelte": "^3.4.0", | ||||||
| "rollup": "^4.44.1", | ||||||
| "rollup-plugin-esbuild-minify": "^1.3.0", | ||||||
| "rollup-plugin-livereload": "^2.0.5", | ||||||
| "rollup-plugin-svelte": "^7.1.0", | ||||||
| "rollup-plugin-terser": "^7.0.2", | ||||||
| "svelte": "^3.42.3", | ||||||
| "svelte-jester": "^2.0.0" | ||||||
| "rollup-plugin-svelte": "^7.2.2", | ||||||
| "svelte": "^5.34.9", | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this should be moved to dependencies |
||||||
| "vitest": "^3.2.4" | ||||||
| }, | ||||||
| "dependencies": { | ||||||
| "single-spa-svelte": "^2.1.1", | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does this version of single-spa-svelte work with svelte@5? svelte should be moved to dependencies.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||||
|
|
||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,17 @@ | ||
| import { defineConfig } from "vite"; | ||
| import { svelte } from "@sveltejs/vite-plugin-svelte"; | ||
|
|
||
| // https://vitejs.dev/config/ | ||
| export default defineConfig({ | ||
| plugins: [svelte()], | ||
| test: { | ||
| globals: true, | ||
| environment: "jsdom", | ||
| setupFiles: ["./setupTests.js"], | ||
| }, | ||
| resolve: process.env.VITEST | ||
| ? { | ||
| conditions: ["browser"], | ||
| } | ||
| : undefined, | ||
| }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why the switch from terser to esbuild? Execution time?
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was no longer maintained. So it seemed prudent to replace it.
Also it required an outdated version of rollup.