feat: use npm run dev-server in docker-compose #31876
Conversation
This configures docker to serve the interactive/dynamic webpack server at `localhost:9000`, and configures it to connect to the backend serve in another container at port 8088. It offers the convenience to auto-webpack and auto-refresh the pages as code is altered/saved.
dosubot
bot
added
change:frontend
| if (parsedArgs.env) { | ||
| return yargs(parsedArgs.env).argv; | ||
| envArgs = yargs(parsedArgs.env).argv; |
There was a problem hiding this comment.
yargs was getting in the way of using normal env vars here
![korbit-ai[bot]](https://avatars.githubusercontent.com/in/322216?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review by Korbit AI
Korbit automatically attempts to detect when you fix issues in new commits.
| Category | Issue | Fix Detected |
|---|---|---|
 |
Avoid unnecessary spreading of process.env. ▹ view | |
 |
Dev Server Proxy Config Not Applied ▹ view | ✅ |
Need a new review? Comment
/korbit-reviewon this PR and I'll review your latest changes.Korbit Guide: Usage and Customization
Interacting with Korbit
- You can manually ask Korbit to review your PR using the
/korbit-reviewcommand in a comment at the root of your PR.- You can ask Korbit to generate a new PR description using the
/korbit-generate-pr-descriptioncommand in any comment on your PR.- Too many Korbit comments? I can resolve all my comment threads if you use the
/korbit-resolvecommand in any comment on your PR.- Chat with Korbit on issues we post by tagging @korbit-ai in your reply.
- Help train Korbit to improve your reviews by giving a 👍 or 👎 on the comments Korbit posts.
Customizing Korbit
- Check out our docs on how you can make Korbit work best for you and your team.
- Customize Korbit for your organization through the Korbit Console.
Feedback and Support
superset-frontend/webpack.config.js
Outdated
| setupMiddlewares: (middlewares, devServer) => { | ||
| // load proxy config when manifest updates | ||
| const { afterEmit } = getCompilerHooks(devServer.compiler); | ||
| afterEmit.tap('ManifestPlugin', manifest => { | ||
| proxyConfig = getProxyConfig(manifest); | ||
| }); | ||
|
|
||
| return middlewares; // Make sure to return the middlewares | ||
| }, |
This comment was marked as resolved.
This comment was marked as resolved.
Sorry, something went wrong.
| } | ||
| return {}; | ||
| return { ...process.env, ...envArgs }; |
There was a problem hiding this comment.
Avoid unnecessary spreading of process.env. 
Tell me more
In the parsedEnvArg function, there is an unnecessary spread of the process.env object. Since process.env is already a global object accessible within the function scope, spreading it into a new object is redundant and wastes memory. To optimize this, consider accessing process.env directly instead of spreading it. Change the return statement to: return { ...envArgs, ...process.env };. This will prioritize variables passed via --env while still allowing access to process.env variables, without the overhead of an extra object spread.
Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help Korbit improve your reviews.
superset-frontend/webpack.config.js
Outdated
| if (isDevMode) { | ||
| config.devServer = { | ||
| onBeforeSetupMiddleware(devServer) { | ||
| setupMiddleware: (middlewares, devServer) => { |
There was a problem hiding this comment.
onBeforeSetupMiddleware had a deprecation warning:
fixed it using a claude-provided solution, looked legit
This configures docker to serve the interactive/dynamic webpack server at
localhost:9000, and configures it to connect to the backend serve in another container at port 8088. It offers the convenience to auto-webpack and auto-refresh the pages as code is altered/saved.