-
-
Notifications
You must be signed in to change notification settings - Fork 282
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
Improve vite experience #17482
base: develop
Are you sure you want to change the base?
Improve vite experience #17482
Conversation
…re that this folder isn't delete when node_modules install is run
ac82621
to
56f31d8
Compare
WalkthroughThe changes involve the integration of the 📜 Recent review detailsConfiguration used: CodeRabbit UI ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
⏰ Context from checks skipped due to timeout of 90000ms (20)
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:
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 (
|
🚀 Expo preview is ready!
|
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!
I did notice it was weird that vite created node_modules
with only .vite
cache in suite-web
, but did not have the capacity to think about it deeper 😆
Yes, now the .vite
cache is placed correctly, and persists when I install yarn
Lint addition OK, from my experience it doesn't bring very noticeable benefit, but why not. There are now gonna be warnings at every PR.. It'd be great if someone fixed them 😉
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.
oh wait, now I noticed that the ESLint warnings fail the pipeline – we introduced that a few months ago.
No need to fix them at once, just add eslint ignore next line and it can be gradually fixed whenever.
👉 Please consider separating it to another PR, because it's thematically unrelated. So it'd be best to extract the eslint addition to a new branch and finish it there. But it's not the end of the world to do it here in a new commit 🤷
Description
yarn
is run, hence delete build cacheRelated Issue
Resolve
Screenshots: