Svelte 4, Vite 5, Jest->Vitest #427
Conversation
- Updates to eslint as required by other updates. There are many eslint errors on main before and after the change and it's not enforced by CI. - Re-enable previously disabled tests.
| "devDependencies": { | ||
| "@babel/preset-env": "^7.23.5", | ||
| "@bulatdashiev/svelte-slider": "^1.0.3", | ||
| "@smui/dialog": "^7.0.0-beta.3", |
There was a problem hiding this comment.
@smui/dialog was entirely unused so I've just removed it
There was a problem hiding this comment.
Might have been an old import. Thank you for the thoroughness :)
| @@ -204,7 +208,7 @@ | |||
| <input | |||
| class="h-25 rotate-90 accent-primary" | |||
| type="range" | |||
| orient="vertical" | |||
| {...noTypeCheckNonStandardOrientProp('vertical')} | |||
There was a problem hiding this comment.
orient is a non-standard prop that seems to have been removed from the type definitions so I've added a workaround
There was a problem hiding this comment.
This has been a pain for a while. It's a shame we have to make this workaround, but the support for vertical range inputs are sorely lacking, and I suspect at some point we will make our own vertical slider to overcome the issue. It will do for now :)
| @@ -84,7 +84,7 @@ | |||
| </p> | |||
| </div> | |||
| {:else} | |||
| <div cla54s="w-3/4 text-primarytext"> | |||
| <div class="w-3/4 text-primarytext"> | |||
There was a problem hiding this comment.
This fix leads to a minor visual change as the width setting now takes effect. As the content is fairly narrow it only takes effect with a narrower window and seems like it was always intended.
| // browser mocks | ||
| const setLang = (lang: string) => { | ||
| const localStorageMock = (function () { | ||
| let store = { | ||
| isTesting: true, | ||
| lang: lang, | ||
| lang: JSON.stringify(lang), |
There was a problem hiding this comment.
This was a bug in the mocks that showed up in the test logs as they expected the language in local storage to be JSON stringified.
| "devDependencies": { | ||
| "@babel/preset-env": "^7.23.5", | ||
| "@bulatdashiev/svelte-slider": "^1.0.3", | ||
| "@smui/dialog": "^7.0.0-beta.3", |
There was a problem hiding this comment.
Might have been an old import. Thank you for the thoroughness :)
| @@ -204,7 +208,7 @@ | |||
| <input | |||
| class="h-25 rotate-90 accent-primary" | |||
| type="range" | |||
| orient="vertical" | |||
| {...noTypeCheckNonStandardOrientProp('vertical')} | |||
There was a problem hiding this comment.
This has been a pain for a while. It's a shame we have to make this workaround, but the support for vertical range inputs are sorely lacking, and I suspect at some point we will make our own vertical slider to overcome the issue. It will do for now :)
| @@ -84,7 +84,7 @@ | |||
| </p> | |||
| </div> | |||
| {:else} | |||
| <div cla54s="w-3/4 text-primarytext"> | |||
| <div class="w-3/4 text-primarytext"> | |||
|
Great job, this is an amazing change |
Although #407 suggested trying Jest/TS upgrades first, I think it's worth the relatively straightforward migration for the maintenance win over time from dropping the extra toolchain. It's at least worth you looking at the extent of the change to see if you're comfortable with it.
Type errors and other incompatibilities pushed me towards upgrading Vite/Svelte to get in a situation compatible with Vitest 1.x. The 3->4 Svelte migration seemed smooth sailing but it would be good for another set of eyes to review the app for unexpected changes.
There's one very minor visual change that I'll comment on inline, as I think it was a bug previously.
Closes #407