Skip to content
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

Migrate to Svelte 4.2.19 #76

Open
wants to merge 10 commits into
base: dev
Choose a base branch
from
Open

Migrate to Svelte 4.2.19 #76

wants to merge 10 commits into from

Conversation

rzats
Copy link
Contributor

@rzats rzats commented Oct 22, 2024

This PR updates the Svelte version used in the project to the stable 4.2.19. It also updates the majority of the project's dependencies to match this new version, with the exceptions being its build tools (rollup & eslint).

It is intended as a prerequisite to #73 and should not be merged until said test suite is finished.

@@ -0,0 +1 @@
21
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The current Node version for Netlify (deploy preview).

@rzats rzats changed the title [WIP] Migrate to Svelte 4.2.19 Migrate to Svelte 4.2.19 Oct 30, 2024
@rzats rzats mentioned this pull request Oct 30, 2024
Copy link
Contributor

@melange396 melange396 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lots of version bumps in package.json! Did you update all those with some tool or by hand? If the latter, are they all just set to the most recent version (as of the time the change was made, at least)?

Comment on lines +106 to +109
constructor(
public readonly title: string,
public readonly datasets: (DataSet | DataGroup)[],
) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks like just whitespace changes... if so, we should exclude this file from the PR.

Comment on lines +6 to +10
constructor(
private readonly year: number,
private readonly month: number,
private readonly day: number,
) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks like just whitespace changes... if so, we should exclude this file from the PR.

Comment on lines +4 to +7
constructor(
private readonly date: EpiDate,
private readonly value: number,
) {}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this looks like just whitespace changes... if so, we should exclude this file from the PR.

Comment on lines +24 to +35
<span
on:click={toggleExpanded}
role="button"
tabindex="0"
on:keydown={(e) => {
if (e.key !== 'Enter' && e.key !== ' ') return;
e.preventDefault();
if (e.target != null) {
e.target.click();
}
}}
>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whats the purpose of this? it looks like an addition in functionality to allow keyboard interaction with the leftmost panel (signal legend), and not a compatibility migration.

Comment on lines +33 to +41
role="button"
tabindex="0"
on:keydown={(e) => {
if (e.key !== 'Enter' && e.key !== ' ') return;
e.preventDefault();
if (e.target != null) {
e.target.click();
}
}}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

whats the purpose of this? it looks like an addition in functionality to allow keyboard interaction with the leftmost panel (signal legend), and not a compatibility migration.

Comment on lines -1 to -2
#!/bin/sh
. "$(dirname "$0")/_/husky.sh"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for posterity, this is removed by/in husky v9.1.1 : typicode/husky#1472 (comment)

@@ -3,7 +3,6 @@
"compilerOptions": {
"module": "ES2020",
"lib": ["ES2019", "DOM"],
"importsNotUsedAsValues": "error",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

for posterity, this is a typescript 5 change: https://stackoverflow.com/questions/75449286/how-to-fix-flag-importsnotusedasvalues-is-deprecated-and-will-stop-functionin

see also addition of the following line below:
"verbatimModuleSyntax": true

@rzats
Copy link
Contributor Author

rzats commented Dec 20, 2024

@melange396 answering all of these at once!

  • I manually applied the package.json updates and tested out the app after each individual change, the versions in there are the latest as of the PR's creation.
  • The whitespace changes in this PR are to fix linting issues, as some new rules were added alongside the package updates.
  • The keyboard interaction changes are also a fix for the linter - specifically, it's a new accessibility requirement to have every button, or custom "button-like" control element, interactable via keyboard.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants