Update NPM dependencies for Svelte, eslint, and eslint plugins #1842
Conversation
This updates Svelte to the latest (5.36) without encountering the Storybook errors we had been running into earlier. The eslint plugin errors do not yet go away, since that fix is apparently in version 3 of eslint-plugin-svelte and we're currently on version 2. Next commit will try the major version upgrade.
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the 📝 WalkthroughWalkthroughThe changes update ESLint configurations and related dependencies in the frontend and viewer projects. ESLint config files are refactored to use the new flat config format, removing backward compatibility utilities. Multiple dependencies and devDependencies are upgraded across several package files, and temporary rule overrides are introduced for Svelte files. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15–20 minutes Suggested labels
Poem
✨ Finishing Touches🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
💻 FW Lite
|
The latest updates on your projects. Learn more about Argos notifications ↗︎
|
Also requires updating eslint.config.js to eslint's new config format, which means we can get rid of the @eslint/eslintrc requirement. Many new lint errors in this commit, due to new lint rules that we don't have ignored yet.
Will fix them in a new PR
|
The failing test for commit eb74380 is installing Chromium build 1169 during the |
|
Updating the Playwright package version fixed the mismatch, and that was the only failure on the previous run. So I expect all the checks to pass now. It'll take 30 minutes for the checks to finish, but I'm going to call this one ready for review now. |
rmunn
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
frontend/viewer/eslint.config.js (1)
158-158: Correct the date in the temporary ignores comment.The comment indicates "2025-07-24 RM" but this appears to be today's date or very recent. Consider updating to reflect when these temporary rules should be revisited.
- // TEMPORARY IGNORES, to be gotten rid of as soon as possible - 2025-07-24 RM + // TEMPORARY IGNORES, to be gotten rid of as soon as possible - added 2025-07-24 RMfrontend/eslint.config.js (1)
147-147: Update the temporary ignores date comment.Same as in the viewer config, the date should be clarified.
- // TEMPORARY IGNORES, to be gotten rid of as soon as possible - 2025-07-24 RM + // TEMPORARY IGNORES, to be gotten rid of as soon as possible - added 2025-07-24 RM
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
frontend/pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (5)
frontend/eslint.config.js(3 hunks)frontend/package.json(4 hunks)frontend/pnpm-workspace.yaml(1 hunks)frontend/viewer/eslint.config.js(3 hunks)frontend/viewer/package.json(1 hunks)
🧰 Additional context used
🧠 Learnings (4)
frontend/viewer/eslint.config.js (1)
Learnt from: rmunn
PR: #1836
File: frontend/viewer/src/lib/components/audio/AudioDialog.svelte:25-25
Timestamp: 2025-07-22T09:19:37.367Z
Learning: In the sillsdev/languageforge-lexbox project, when file size limits or other constants need to be shared between C# backend and TypeScript frontend code, prefer exposing them through Reinforced.Typings type generation rather than hardcoding the values separately. This ensures consistency and prevents discrepancies when values change.
frontend/pnpm-workspace.yaml (1)
Learnt from: rmunn
PR: #1779
File: frontend/package.json:5-5
Timestamp: 2025-06-26T08:50:59.313Z
Learning: In PNPM projects, it's acceptable to have a packageManager field specifying a higher version (e.g., [email protected]) while keeping engines.pnpm at a lower minimum (e.g., >=9) for backward compatibility. PNPM 9 will automatically run the version specified in packageManager, allowing developers to use newer PNPM features without forcing immediate upgrades.
frontend/eslint.config.js (1)
Learnt from: rmunn
PR: #1836
File: frontend/viewer/src/lib/components/audio/AudioDialog.svelte:25-25
Timestamp: 2025-07-22T09:19:37.367Z
Learning: In the sillsdev/languageforge-lexbox project, when file size limits or other constants need to be shared between C# backend and TypeScript frontend code, prefer exposing them through Reinforced.Typings type generation rather than hardcoding the values separately. This ensures consistency and prevents discrepancies when values change.
frontend/package.json (1)
Learnt from: rmunn
PR: #1779
File: frontend/package.json:5-5
Timestamp: 2025-06-26T08:50:59.313Z
Learning: In PNPM projects, it's acceptable to have a packageManager field specifying a higher version (e.g., [email protected]) while keeping engines.pnpm at a lower minimum (e.g., >=9) for backward compatibility. PNPM 9 will automatically run the version specified in packageManager, allowing developers to use newer PNPM features without forcing immediate upgrades.
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
- GitHub Check: GHA integration tests / dotnet
- GitHub Check: GHA integration tests / playwright
- GitHub Check: Build FW Lite and run tests
🔇 Additional comments (7)
frontend/viewer/package.json (1)
34-111: Dependency updates look good.The version bumps are primarily minor and patch updates, which should maintain backward compatibility. The updates align with the PR's objective to update NPM dependencies.
frontend/pnpm-workspace.yaml (1)
2-29: Catalog updates align with PR objectives.Good changes:
- Svelte updated to 5.36.14 as intended
- eslint-plugin-svelte upgraded to v3 to resolve the mentioned plugin errors
- Addition of
typescript-eslintpackage supports the ESLint config modernization- Removing quotes from package names improves consistency
frontend/package.json (2)
110-110: Why pin sveltekit-search-params to exact version?The change from
^3.0.0to3.0.0removes the ability to receive patch updates. Was this intentional to avoid potential breaking changes?
34-80: Dependency updates are consistent with workspace changes.The addition of
typescript-eslintand other version bumps align well with the ESLint configuration modernization effort.frontend/viewer/eslint.config.js (1)
8-48: Good modernization to flat config format.The removal of FlatCompat and direct use of the new flat config format is a positive improvement that aligns with current ESLint best practices.
frontend/eslint.config.js (2)
8-48: Consistent modernization with viewer config.The flat config adoption is well-executed and maintains consistency across the frontend codebase.
146-153: Different temporary rule overrides compared to viewer config.This config only disables 2 Svelte rules while the viewer config disables 4. Consider documenting why different rules need to be disabled in each project, or align them for consistency if the same issues exist in both.
|
Replying to @coderabbitai comments here since GitHub doesn't seem to be displaying a conversation-thread reply box for them.
This style of signing comments with today's date and the initials of the commenter is something we use often in our organization, allowing people to see when the comment was added. Inserting the word "added" is redundant in this case because that's going to be understood by anyone who reads this.
Same comment as above: the word "added" is redundant because in our organization, when we sign and date comments it means "this is when the comment was added" rather than "this is a future date when something should be done about this comment".
CodeRabbit is mistaken, the
No need to document this since this override is clearly marked as temporary. We will be removing the override as soon as we fix the errors, which should be soon. |
|
Tip For best results, initiate chat on the files or code changes. 🧩 Analysis chain@rmunn Thank you for the clarifications about your organization's practices and for the corrections! You're absolutely right about the comment signing style - I can see that the date and initials format is a clear organizational convention for indicating when comments were added. I'll keep this in mind for future reviews. Regarding the For the temporary rule overrides, I understand that since they're explicitly marked as temporary and will be addressed soon, there's no need for additional documentation explaining the differences between the two configs. Thank you for taking the time to explain these details - it helps me provide more relevant feedback in the future. 🐰 🏁 Script executed: Length of output: 276 You're absolutely correct about the Thank you for taking the time to clarify these points. Your explanations about the organizational practices and the correction about the package version help me provide more accurate feedback in future reviews. 🐰 ✏️ Learnings added
|
…-pkgs # Conflicts: # frontend/pnpm-lock.yaml # frontend/viewer/package.json
|
Looks good to me in testing; I can't get it to produce any errors, and everything in FW Lite looks the way it's supposed to. |
3 participants
Fixes #1840.
This PR updates the eslint plugins for Svelte and Typescript to get rid of the spurious eslint warnings we've been seeing in FW Lite. It also updates Storybook and Svelte to their latest versions (5.36 in the case of Svelte, which will allow the use of
$derived(await someAsyncFunction())) without encountering the Storybook errors we had run into in our previous attempt to upgrade Svelte.The eslint plugin package updates create new errors (due to new checks in the Svelte eslint plugin), so we temporarily disable those errors so that this PR will not break the build. We'll fix those errors (or decide on an individual basis whether to ignore them) in another PR.