-
-
Notifications
You must be signed in to change notification settings - Fork 59
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?
Conversation
internettrans
left a comment
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.
This pull request changes a lot more more than in the title/description.
Please add changeset(s) per the CONTRIBUTING.md describing the changes.
| "vitest": "^3.2.4" | ||
| }, | ||
| "dependencies": { | ||
| "single-spa-svelte": "^2.1.1", |
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.
Does this version of single-spa-svelte work with svelte@5?
svelte should be moved to dependencies.
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.
| "single-spa-svelte": "^2.1.1", | |
| "single-spa-svelte": "^3.0.0-beta.0", |
| "svelte": "^3.42.3", | ||
| "svelte-jester": "^2.0.0" | ||
| "rollup-plugin-svelte": "^7.2.2", | ||
| "svelte": "^5.34.9", |
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.
this should be moved to dependencies
| @@ -1,5 +1,5 @@ | |||
| <script> | |||
| export let name; | |||
| <script lang="ts"> | |||
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.
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.
| @@ -0,0 +1 @@ | |||
| import "@testing-library/jest-dom/vitest"; | |||
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.
Pretty weird that @testing-library/jest-dom includes an implementation for vitest, since jest and vitest are competitor libraries.
| output: { | ||
| sourcemap: true, | ||
| format: "system", | ||
| format: "esm", |
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.
This is a breaking change, and should be reflected in changesets.
| 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"; |
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?
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.
| "start": "rollup -c -w", | ||
| "serve": "sirv dist -c", | ||
| "test": "jest", | ||
| "test": "vitest", |
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.
Did the jest configuration no longer work after the upgrade to svelte? Why switch from jest to vitest?
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.
To simplify the test setup. Jest requires some hurdles to go through with ESM.
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.
I'm ok switching to vitest for this generator. Jest typescript esm with node flags for vm modules is a pain.
| "prettier": "^2.3.2", | ||
| "prettier-plugin-svelte": "^2.3.1", | ||
| "rollup": "^2.56.3", | ||
| "@babel/core": "^7.27.7", |
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.
The babel.config.js was deleted - was it only being used by jest? If so, are @babel/core and @babel/preset-env being used anymore?
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.
Yes, I forgot to remove them. Will do so.
| @@ -1,33 +1,33 @@ | |||
| { | |||
| "name": "<%= name %>", | |||
| "type": "module", | |||
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.
This is a breaking change and should be described in a changeset.
internettrans
left a comment
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.
Have you verified that the generated svelte microfrontend works?
| "vitest": "^3.2.4" | ||
| }, | ||
| "dependencies": { | ||
| "single-spa-svelte": "^2.1.1", |
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.
| "single-spa-svelte": "^2.1.1", | |
| "single-spa-svelte": "^3.0.0-beta.0", |
Sorry for the late reaction. I've just made some final adjustments and verified that it works. |
Upgrade outdated versions of Svelte and other packages. Simplify test setup by using Vitest instead of Jest.
Developed in conjunction with single-spa/single-spa-svelte#34