fix: fix windows build command basign on --mode [mode] flag#5841
Conversation
Summary of ChangesHello @sohaieb, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request resolves a critical build failure on Windows environments by refactoring how environment variables are passed to Vite commands. Instead of directly injecting Highlights
Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request correctly resolves a build issue on Windows by adopting Vite's --mode flag instead of setting NODE_ENV directly in npm scripts. This change ensures cross-platform compatibility for the build and development commands. The vite.config.ts is also properly updated to use a function configuration, accessing the mode passed by Vite. The implementation is clean and effective.
|
|
||
| const isDev = process.env.NODE_ENV === 'development' | ||
| export default defineConfig(({ mode }) => { | ||
| const isDev = mode === 'development' |
There was a problem hiding this comment.
based on the doc you shared https://vite.dev/guide/env-and-mode#modes, we don't even need to pass in mode since build script will by default be production
There was a problem hiding this comment.
Hi
You are completely right @jocelynlin-wd , thank you for the catch !
Basing on the following announced remark in Vite - Env and Mode Doc:
By default, the dev server (dev command) runs in development mode and the build command runs in production mode.
I will keep the code above as we still need it to specify the sourceMap config according to the environment/mode, but I will adjust the package.json scripts instead.
There was a problem hiding this comment.
Hi again @jocelynlin-wd ,
I made the updates according to our discussion,
In the meanwhile, another topic: I created another PR to add LMStudio support too and it fixes some UI issues.
I would really appreciate a lot if you review both PRs 😃
Thank you so much 🙏
7e47d85 to
3fb8d3a
Compare
3fb8d3a to
1893870
Compare
|
Great idea. Thanks for bringing this up and putting in a better environment check than what I had originally 🙏 |
jocelynlin-wd
left a comment
There was a problem hiding this comment.
thanks for the fix
thank you very much !
It's really a pleasure to collaborate! |
1bac5fb to
175d1c5
Compare
553f770 to
b7e87f7
Compare
pnpm buildandpnpm build-forcecommands on Windows systems (due to injectingNODE_ENVvariable directly in the command line.cross-envpackage from the repo