Skip to content

Conversation

carylwyatt
Copy link
Member

@carylwyatt carylwyatt commented Nov 25, 2024

upgrade to svelte 4

I followed the official migration guide and had no issues. Only a handful of components were affected and they were superficial changes (css transitions), so there would be no breaking changes, regardless. But I checked those components locally and on dev-3, and they seem to be fine!

upgrade to storybook 8

I used the migration helper tool to upgrade to version 8. There were a few breaking changes, and the migration tool checked the story files for those changes and helped me make necessary changes. There were a few issues, but they were easily remedied.

remove warnings

If you've ever built firebird during testing/staging/deploying, you may have noticed a long list of annoying warnings. Those warnings were not helpful and often made it difficult to see if an error was mixed in with the long wall of text. I went through various components and removed warnings by either specifically telling svelte to ignore a known problem or fixing the problem. I also updated the vite config (in both firebird and storybook) to exclude fonts/images that are usually found in the /common/dist folder but vite has a hard time finding them during build time because they're relative links (this doesn't affect how the components/apps work, it's just a compile warning). A few warnings still remain, but I hope to resolve those in the future.

package audit fixes

There were three packages that needed updating according to npm audit. Those were upgraded without issue.

Thanks to @liseli for working through this with me during Maintenance Monday! 🥇

@carylwyatt carylwyatt requested a review from liseli November 25, 2024 19:00
Copy link

netlify bot commented Nov 25, 2024

Deploy Preview for hathitrust-firebird-common failed.

Name Link
🔨 Latest commit e0a8a75
🔍 Latest deploy log https://app.netlify.com/sites/hathitrust-firebird-common/deploys/6744c926219dac000801ca3d

Copy link

@liseli liseli left a comment

Choose a reason for hiding this comment

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

The changes on this PR sees fine. However, the deploy failed, then you should check the fails before merge it. We could probably check the fails during the maintenance co-working time

@carylwyatt
Copy link
Member Author

The netlify build is using an old version of node, but storybook version 8 needs node 18+ to build properly. We don't actually use netlify for anything anymore, and I don't have access to it. @respinos I know this is used as a fallback for js/css across the apps, but we don't really need it, right?

@respinos
Copy link
Contributor

respinos commented Dec 2, 2024

@carylwyatt nope --- I can delete the netlify configuration. Y'all can then recreate it as needed.

@carylwyatt
Copy link
Member Author

Thanks, @respinos! 🙌

@carylwyatt carylwyatt merged commit cc81915 into main Dec 16, 2024
6 of 10 checks passed
@carylwyatt carylwyatt deleted the upgrade-svelte-4 branch December 16, 2024 16:37
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.

3 participants