-
Notifications
You must be signed in to change notification settings - Fork 41
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
Bump vite to 5.4.14
#2312
Bump vite to 5.4.14
#2312
Conversation
Update: I recreated the lock file without deduping Just fyi, I tried to dedupe just
|
Can you clarify which versions of |
Ohh, I see. The versions in the package-lock are: I kept the title as-is for now even though |
PR changed significantly since approval.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just make sure to update PR description
I'm noticing CI is failing for react visual tests, but I think that's caused by #2391; maybe one of the Toast images needs to be updated to be less flaky.
Sounds good, updated the PR description. Also updated the PR title for clarity.
Ohh, I see. Will take a look. Added it as an after PR TODO in #2391. |
Changes
After PR TODO from #2256 (#2256 (comment)).
Fixes GHSA-vg6x-rcgg-rjx6. Closed #2409 in favor of this PR.
Bumps
vite
from~5.1
to~5.4.14
(after deduping, only5.4.14
is used atm).After the bump, the
styles.css
was not getting generated instyles.js/dist
. Looks like it is because of vitejs/vite/#16051 from 5.2.0-beta.0.To fix it, I added the vite plugin suggested in vitejs/vite#16051 (comment). I had tried it when I had opened this PR too. However, it didn't work back then (which is why I had tried another fix which posed other problems; more info below). But since this vite plugin works now, I went ahead with this.
Old fix
To fix it, I renamed
styles.module.css
tostyles.css
. If there is a reason why the file has to be amodule.css
file, then we'd have to import and use the styles somewhere, or something along these lines.One problem with using
styles.css
, however, is that theiui-
prefix is not there in the generated styles.css file because the css module part of the vite config file is not applied.Since I noticed multiple
vite
versions, especially some that were still in the affected range of the security advisory, I also deduped it to only use5.4.14
(b50db2a). But this caused the css-workshop tests to all fail. To fix it, I changed the test script to replaceastro preview
withserve
and added await-on
step so that the tests start up only afterserve
has started up the server. (more info).Testing
build
script is working in theitwinui-react
workspacestyles.css
inpackages/itwinui-react
is unchanged.Docs
No changeset as
vite
is used in dev workspaces or listed asdevDependencies
in the package workspaces (itwinui-react
).