Housekeeping: Dependency updates - #316
Conversation
davinotdavid
left a comment
davinotdavid
left a comment
There was a problem hiding this comment.
Can we also double check that every package that got bumped to a major version didn't have any breaking changes?
| "@eslint/js": "9.39.4", | ||
| "@playwright/test": "1.61.1", | ||
| "@eslint/js": "10.0.1", | ||
| "@microsoft/api-extractor": "^7.59.0", |
There was a problem hiding this comment.
This doesn't seem to be used anywhere and is a net new dependency added. Why has this been added?
There was a problem hiding this comment.
api-extractor is needed by vite-plugin-dts v5 now (in v4 it was bundled internally) as optional peer dependency. Without installing it explicitly, bundling silently failed and fell back to unbundled per-file declarations (breaking our dist/index.d.ts).
There was a problem hiding this comment.
Hmm I see, interesting. Perhaps we should remove the caret here / pin the version!
Also, noting here that this package seems to bundle its own version of typescript (which is 5.93 according to their package.json) that gives us a warning when building our project since our typescript version is now 6.x.
We don't usually use the latest features of typescript anyways but perhaps we should watch out and update this again when api-extractor updates their bundled typescript version.
There was a problem hiding this comment.
Good catch, I saw that too. But I saw this more like a note for api-extractor to update TS at their end - or am I getting that wrong? Since we already have TS v7 available, I thought going at least to v6 would be sufficient for now. All builds worked on my end locally, but please test this too 🙏🏻 Otherwise, I can happily revert to TS v5, this was only the baseUrl change.
There was a problem hiding this comment.
Oh and good point on the caret! Will remove it!
There was a problem hiding this comment.
Yes that's what I meant as well. Whenever api-extractor updates their TS version and release a new version with it, we should update it here as well (note for the future).
| // The root vite config's dts plugin only makes sense for the library build, not for storybook-static. | ||
| // It fails there since api-extractor can't resolve an entry file under storybook's own outDir. | ||
| viteFinal: async (config) => { | ||
| config.plugins = config.plugins?.filter( | ||
| (plugin) => !(plugin && 'name' in plugin && plugin.name === 'unplugin-dts'), | ||
| ); | ||
| return config; | ||
| }, |
There was a problem hiding this comment.
Can you elaborate on this? What is and where are we using an api-extractor?
There was a problem hiding this comment.
Yes, this one is tricky because we're doing separate things in the same repository here. We have builds for the main component library (the published npm package), we have the web components build and we have the static storybook site. All these sharing the same config more or less implicitly.
@storybook/vue3-vite auto-merges our root vite.config.ts, so the dts() plugin meant only for the main library build was always leaking into the Storybook build too. Under v4 it just silently wrote an additional unused file to that build, but v5's bundleTypes computes the entry path from Storybook's actual outDir and hard-fails when it doesn't exist. So this filter makes sure the plugin is excluded for storybook.
There was a problem hiding this comment.
We could maybe also just use a dedicated vite config für storybook? 🤔 Maybe that would be cleaner, I'll take a look at that. That would duplicate some config we have, but we don't have to do filter magic.
There was a problem hiding this comment.
IMHO we can leave it for now. Thanks for the explanations! We can trace it back to this discussion if we want a bit more clarity on why that is and even though we have separate things in the same repository, we still (thankfully) don't have an overly complicated setup (yet).
There was a problem hiding this comment.
You're welcome, thanks for asking. Alright, let's keep it as is.
|
@davinotdavid Of course. Here are the breaking changes of the major upgrades I did:
|
2 participants
What changed?
This PR contains lots of dependency updates, housekeeping and clean-up. This is a second try after #274 failed.
vue-tscandtypescript. Those are not (yet) compatible with us using the standard Storybook pattern of spying on array-styledefineEmits(['click'])withonClick/onChangedargsvite-plugin-dtshad leaked its dts() plugin into the Storybook build, because@storybook/vue3-viteauto-merges the project's root vite.config.ts — we never scoped this to the library build only.Why?
Fixing security issues. Housekeeping.
Limitations and Notes
See the comments above.
Applicable Issues
Closes #273
Screenshots
No UI changes.