codeant-ai-for-open-source[bot] commented on code in PR #36433:
URL: https://github.com/apache/superset/pull/36433#discussion_r2724522504
##########
superset-frontend/webpack.config.js:
##########
@@ -219,16 +219,13 @@ if (!isDevMode) {
);
}
-const PREAMBLE = [path.join(APP_DIR, '/src/preamble.ts')];
-if (isDevMode) {
- // A Superset webpage normally includes two JS bundles in dev, `theme.ts` and
- // the main entrypoint. Only the main entry should have the dev server
client,
- // otherwise the websocket client will initialize twice, creating two
sockets.
- // Ref: https://github.com/gaearon/react-hot-loader/issues/141
- PREAMBLE.unshift(
- `webpack-dev-server/client?http://localhost:${devserverPort}`,
- );
-}
+// In dev mode, include theme.ts in preamble to avoid separate chunk HMR issues
+const PREAMBLE = isDevMode
+ ? [
+ path.join(APP_DIR, '/src/theme.ts'),
+ path.join(APP_DIR, '/src/preamble.ts'),
+ ]
+ : [path.join(APP_DIR, '/src/preamble.ts')];
Review Comment:
**Suggestion:** Using absolute paths with a leading slash in
`path.join(APP_DIR, '/src/...')` causes `path.join` to ignore `APP_DIR` and
resolve to `/src/...`, so the `PREAMBLE` entries point to non-existent files
and will make webpack fail to resolve the preamble modules. [logic error]
<details>
<summary><b>Severity Level:</b> Critical 🚨</summary>
```mdx
- ❌ Dev HMR fails due to unresolved preamble modules.
- ❌ Dev server build breaks on startup.
- ⚠️ Local development workflow blocked until fixed.
```
</details>
```suggestion
path.join(APP_DIR, 'src/theme.ts'),
path.join(APP_DIR, 'src/preamble.ts'),
]
: [path.join(APP_DIR, 'src/preamble.ts')];
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Start dev server with this PR changes: run `npm run dev-server` (dev
server entry
configured in `superset-frontend/webpack.config.js`).
2. When isDevMode is true, webpack config builds PREAMBLE at
`superset-frontend/webpack.config.js:223` (const PREAMBLE = isDevMode ...).
3. Current code calls `path.join(APP_DIR, '/src/theme.ts')` and
`path.join(APP_DIR,
'/src/preamble.ts')` (lines 224-226). A leading slash in the second argument
causes Node's
path.join to ignore APP_DIR and resolve to '/src/...', so the resolved
preamble paths
point outside the repo.
4. Webpack later uses PREAMBLE entries as entry modules (see config.entry at
`superset-frontend/webpack.config.js:294-301`). Because the preamble paths
are incorrect,
module resolution fails with "Module not found" errors when webpack attempts
to include
theme.ts / preamble.ts during dev builds.
5. Reproducing locally: run `npm run dev-server` with this file; observe
webpack error
logs complaining it cannot resolve "/src/preamble.ts" (module resolution
failure
originating from the PREAMBLE entries at lines 223-228).
6. The proposed change at lines 223-228 (remove leading slash: use
'src/...') fixes
path.join behavior so entries resolve to actual files under APP_DIR and
eliminates the
module not found error.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/webpack.config.js
**Line:** 225:228
**Comment:**
*Logic Error: Using absolute paths with a leading slash in
`path.join(APP_DIR, '/src/...')` causes `path.join` to ignore `APP_DIR` and
resolve to `/src/...`, so the `PREAMBLE` entries point to non-existent files
and will make webpack fail to resolve the preamble modules.
Validate the correctness of the flagged issue. If correct, How can I resolve
this? If you propose a fix, implement it and please make it concise.
```
</details>
--
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]
