Upgrade Svelte to v5. Replace Jest with Vitest. Upgrade other packages - #34
Conversation
internettrans
left a comment
internettrans
left a comment
There was a problem hiding this comment.
What version(s) of svelte does this code work with? All versions of svelte 5? Does it work with svelte 4?
The github workflow should be updated to latest versions, too
| // JSDom + Vitest don't play well with each other. Long story short - default | ||
| // TextEncoder produces Uint8Array objects that are _different_ from the global | ||
| // Uint8Array objects, so some functions that compare their types explode. | ||
| // https://github.com/vitest-dev/vitest/issues/4043#issuecomment-1905172846 |
There was a problem hiding this comment.
Does the following comment also fix the issue? It is a simpler workaround
There was a problem hiding this comment.
I could not get it to work with that solution.
| @@ -5,7 +5,7 @@ | |||
| "main": "lib/single-spa-svelte.js", | |||
| "scripts": { | |||
| "build": "rimraf lib && babel src/single-spa-svelte.js --out-file lib/single-spa-svelte.js --source-maps", | |||
There was a problem hiding this comment.
Babel is still being used here - why was the .babelrc removed?
There was a problem hiding this comment.
My mistake, reverting
There was a problem hiding this comment.
babel is still missing from devDependencies after the latest commits. I'd like to also change the package.json to use exports rather than main, with type: "module", but can do that in a follow-up pull request if you do not have the time to do so in this one. The module format of the code in the lib folder should be ESM
There was a problem hiding this comment.
I don't have time to do so at the moment, so have to add it a later point.
| . "$(dirname "$0")/_/husky.sh" | ||
|
|
||
| pnpx pretty-quick --staged && pnpx lint | ||
| pnpm pretty-quick --staged && pnpm run lint |
There was a problem hiding this comment.
| pnpm pretty-quick --staged && pnpm run lint | |
| pnpm exec pretty-quick --staged && pnpm run lint |
Only svelte 5. If you want it to continue working with the older versions I'll take another go at it. Can you point me to the github workflow? |
internettrans
left a comment
internettrans
left a comment
There was a problem hiding this comment.
github workflow: The node-version and pnpm/action-setup should be updated to latest versions. The version of pnpm to use should be specified in the packageManager field of package.json, which pnpm/action-setup uses automatically. Should use latest version.
Since this is a breaking change to single-spa-svelte, I'd like to also migrate it to typescript in the process. If you don't have the time to do so in this pull request, I will do so in a follow-up.
| @@ -0,0 +1,21 @@ | |||
| // JSDom + Vitest don't play well with each other. Long story short - default | |||
There was a problem hiding this comment.
This file should be moved to src folder.
| @@ -1,3 +1,5 @@ | |||
| import { mount as originalMount, unmount as originalUnmount } from "svelte"; | |||
There was a problem hiding this comment.
svelte is in devDependencies, which means that this import would fail in a library that installs single-spa-svelte. Traditionally, single-spa adapter libraries for frameworks use dependency injection of the framework (pass in React/Angular/Vue libs via opts) rather than imports, due to the complexities of bundling the adapter libraries and/or using them as runtime shared dependencies in import maps.
Please switch to the mount and unmount functions being passed in as opts
There was a problem hiding this comment.
Added mount and unmount options
| @@ -5,7 +5,7 @@ | |||
| "main": "lib/single-spa-svelte.js", | |||
| "scripts": { | |||
| "build": "rimraf lib && babel src/single-spa-svelte.js --out-file lib/single-spa-svelte.js --source-maps", | |||
There was a problem hiding this comment.
babel is still missing from devDependencies after the latest commits. I'd like to also change the package.json to use exports rather than main, with type: "module", but can do that in a follow-up pull request if you do not have the time to do so in this one. The module format of the code in the lib folder should be ESM
|
Build failed because of pnpm lockfile being out of date. Github used to let me push to pull request branches but doesn't anymore. I am going to merge this as-is and then complete the rewrite. |
2 participants
Upgrade outdated versions of Svelte and other packages. Simplify test setup by using Vitest instead of Jest.
Developed in conjunction with single-spa/create-single-spa#460