korbit-ai[bot] commented on code in PR #36041:
URL: https://github.com/apache/superset/pull/36041#discussion_r2504648182


##########
superset-frontend/webpack.config.js:
##########
@@ -181,6 +182,11 @@ if (!process.env.CI) {
   plugins.push(new webpack.ProgressPlugin());
 }
 
+// Add React Refresh plugin for development mode
+if (isDevMode) {
+  plugins.push(new ReactRefreshWebpackPlugin());
+}

Review Comment:
   ### Unconditional React Refresh plugin in dev mode <sub>![category 
Performance](https://img.shields.io/badge/Performance-4f46e5)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   ReactRefreshWebpackPlugin is added unconditionally in development mode 
without checking if hot module replacement is enabled or if the dev server is 
running.
   
   
   ###### Why this matters
   This could cause unnecessary overhead and potential conflicts when React 
Refresh is not needed, such as when building for development without the dev 
server or when HMR is disabled.
   
   ###### Suggested change ∙ *Feature Preview*
   Add a condition to check if the dev server is running before adding the 
ReactRefreshWebpackPlugin:
   
   ```javascript
   if (isDevMode && isDevServer) {
     plugins.push(new ReactRefreshWebpackPlugin());
   }
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bfdefc95-1875-4301-8b27-a077b4e8fdc8/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bfdefc95-1875-4301-8b27-a077b4e8fdc8?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bfdefc95-1875-4301-8b27-a077b4e8fdc8?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bfdefc95-1875-4301-8b27-a077b4e8fdc8?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bfdefc95-1875-4301-8b27-a077b4e8fdc8)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:7ea96faa-7765-4128-840b-d3cc1342e5a1 -->
   
   
   [](7ea96faa-7765-4128-840b-d3cc1342e5a1)



##########
superset-frontend/webpack.config.js:
##########
@@ -181,6 +182,11 @@ if (!process.env.CI) {
   plugins.push(new webpack.ProgressPlugin());
 }
 
+// Add React Refresh plugin for development mode
+if (isDevMode) {
+  plugins.push(new ReactRefreshWebpackPlugin());
+}

Review Comment:
   ### Missing SWC-React Refresh integration <sub>![category 
Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   ReactRefreshWebpackPlugin is added without proper configuration for SWC 
loader integration, which may cause React Fast Refresh to not work correctly.
   
   
   ###### Why this matters
   The plugin requires the SWC loader to have refresh: true in its 
transform.react configuration to enable Fast Refresh functionality. Without 
this coordination, hot reloading may fail or behave inconsistently during 
development.
   
   ###### Suggested change ∙ *Feature Preview*
   The SWC loader configuration already has `refresh: isDevMode` in the 
`transform.react` section, so the integration is actually correct. However, to 
ensure proper functionality, verify that the ReactRefreshWebpackPlugin options 
are compatible with SWC by adding explicit configuration:
   
   ```javascript
   if (isDevMode) {
     plugins.push(new ReactRefreshWebpackPlugin({
       overlay: false, // Let webpack-dev-server handle overlays
     }));
   }
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6d24fd2a-540c-4369-9d09-c765b89135e2/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6d24fd2a-540c-4369-9d09-c765b89135e2?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6d24fd2a-540c-4369-9d09-c765b89135e2?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6d24fd2a-540c-4369-9d09-c765b89135e2?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6d24fd2a-540c-4369-9d09-c765b89135e2)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:775343ec-eb34-436e-9ffe-a691d6be19ae -->
   
   
   [](775343ec-eb34-436e-9ffe-a691d6be19ae)



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to