You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by "msyavuz (via GitHub)" <gi...@apache.org> on 2025/11/07 17:08:44 UTC
[PR] fix(webpack): webpack refresh plugin [superset]
msyavuz opened a new pull request, #36041:
URL: https://github.com/apache/superset/pull/36041
<!---
Please write the PR title following the conventions at https://www.conventionalcommits.org/en/v1.0.0/
Example:
fix(dashboard): load charts correctly
-->
### SUMMARY
<!--- Describe the change below, including rationale and design decisions -->
After migrating to webpack with swc dev-server failed with hot reload errors. I added the necessary webpack plugins to make hot reload work with swc in this pr.
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
<!--- Skip this if not applicable -->
### TESTING INSTRUCTIONS
<!--- Required! What steps can be taken to manually verify the changes? -->
`npm run dev-server`should run and app should work without webpack errors.
### ADDITIONAL INFORMATION
<!--- Check any relevant boxes with "x" -->
<!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(webpack): webpack refresh plugin [superset]
Posted by "rusackas (via GitHub)" <gi...@apache.org>.
rusackas merged PR #36041:
URL: https://github.com/apache/superset/pull/36041
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(webpack): webpack refresh plugin [superset]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #36041:
URL: https://github.com/apache/superset/pull/36041#discussion_r2504926296
##########
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:
React Refresh is already enabled in the SWC loader configuration at line 255 (`refresh: isDevMode`). Adding the ReactRefreshWebpackPlugin creates a redundant configuration that may cause conflicts. SWC's built-in React Refresh support is sufficient and doesn't require the additional webpack plugin. Consider removing this plugin instantiation or disabling the SWC refresh option if you specifically need to use the webpack plugin approach.
```suggestion
// React Refresh is handled by SWC; do not add ReactRefreshWebpackPlugin to avoid conflicts.
```
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(webpack): webpack refresh plugin [superset]
Posted by "korbit-ai[bot] (via GitHub)" <gi...@apache.org>.
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></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
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bfdefc95-1875-4301-8b27-a077b4e8fdc8/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bfdefc95-1875-4301-8b27-a077b4e8fdc8?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bfdefc95-1875-4301-8b27-a077b4e8fdc8?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/bfdefc95-1875-4301-8b27-a077b4e8fdc8?what_not_in_standard=true) [](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></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
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6d24fd2a-540c-4369-9d09-c765b89135e2/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6d24fd2a-540c-4369-9d09-c765b89135e2?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6d24fd2a-540c-4369-9d09-c765b89135e2?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6d24fd2a-540c-4369-9d09-c765b89135e2?what_not_in_standard=true) [](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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
Re: [PR] fix(webpack): webpack refresh plugin [superset]
Posted by "bito-code-review[bot] (via GitHub)" <gi...@apache.org>.
bito-code-review[bot] commented on PR #36041:
URL: https://github.com/apache/superset/pull/36041#issuecomment-3503718631
AI Code Review is in progress (usually takes 3 to 15 minutes unless it's a very large PR).
--
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: notifications-unsubscribe@superset.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org