codeant-ai-for-open-source[bot] commented on code in PR #37588:
URL: https://github.com/apache/superset/pull/37588#discussion_r2749402633
##########
docs/src/webpack.extend.ts:
##########
@@ -77,25 +77,35 @@ export default function webpackExtendPlugin(): Plugin<void>
{
test: /\.(tsx?|jsx?)$/,
include: supersetFrontendPath,
use: {
- loader: 'babel-loader',
+ loader: 'swc-loader',
Review Comment:
**Suggestion:** The new swc-loader rule includes the entire
`superset-frontend` tree, which also contains its `node_modules`; this causes
third-party dependencies to be re-transpiled with the TypeScript parser and
custom SWC settings, risking parse/runtime errors (e.g. for libraries using
non-TypeScript syntax like Flow) and significantly degrading build performance.
To avoid this, the rule should explicitly exclude `node_modules` under
`superset-frontend` so only the project source files are processed. [possible
bug]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Local docs dev build can fail parsing third-party modules.
- ❌ Docs site may not start at localhost:3000.
- ⚠️ Build performance degraded due to re-transpiling node_modules.
- ⚠️ Increased memory/CPU during incremental rebuilds.
```
</details>
```suggestion
exclude: /node_modules/,
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the docs webpack extension at docs/src/webpack.extend.ts and locate
the SWC rule
(file shown in PR final state). The SWC rule is the block starting at line 77
(config.module?.rules?.push({...})) through line 99 (});).
2. Run the docs dev server (yarn start in docs/) which triggers Docusaurus
webpack config
to be extended using this file. During compilation, webpack evaluates rules
and applies
this SWC rule to files under the included path (supersetFrontendPath
resolved at lines
~72-76 in the same file).
3. Because the rule only uses include: supersetFrontendPath and does not
exclude
superset-frontend/node_modules, webpack will pass third-party modules under
superset-frontend/node_modules (e.g. packages that use Flow, plain JS, or
non-TS syntax)
through the SWC TypeScript parser at the jsc.parser configuration (syntax:
'typescript',
tsx: true) defined at lines ~85-93.
4. Observe one of two real outcomes:
- Parse/runtime errors during startup (SWC throws on Flow-only or
unsupported syntax),
failing the docs build and preventing localhost:3000 from loading.
- Much slower build and repeated transpilation of node_modules, causing
increased
memory and CPU usage and negating the intended build-performance
improvements.
5. Confirm by inspecting the webpack module processing logs (or enabling
verbose loader
logs): files under ../../superset-frontend/node_modules/* will be matched
and compiled by
swc-loader because no exclude is present. Reproducing these symptoms
requires no special
input — just a normal yarn start that resolves superset-frontend with its own
node_modules.
Explanation: The suggested change (adding exclude: /node_modules/) prevents
transpiling
third-party packages. Given the file context and how webpack applies rules
(as shown in
docs/src/webpack.extend.ts), this is a concrete, reachable issue during
normal local
development.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** docs/src/webpack.extend.ts
**Line:** 80:80
**Comment:**
*Possible Bug: The new swc-loader rule includes the entire
`superset-frontend` tree, which also contains its `node_modules`; this causes
third-party dependencies to be re-transpiled with the TypeScript parser and
custom SWC settings, risking parse/runtime errors (e.g. for libraries using
non-TypeScript syntax like Flow) and significantly degrading build performance.
To avoid this, the rule should explicitly exclude `node_modules` under
`superset-frontend` so only the project source files are processed.
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>
##########
docs/docusaurus.config.ts:
##########
@@ -171,9 +171,11 @@ const config: Config = {
url: 'https://superset.apache.org',
baseUrl: '/',
onBrokenLinks: 'warn',
- onBrokenMarkdownLinks: 'throw',
markdown: {
mermaid: true,
+ hooks: {
+ onBrokenMarkdownLinks: 'throw',
+ },
Review Comment:
**Suggestion:** The `onBrokenMarkdownLinks` setting is nested under a
`hooks` object inside `markdown`, which Docusaurus does not read for this
option, so broken Markdown links will not actually cause a throw as intended;
instead, place `onBrokenMarkdownLinks` directly on the `markdown` object so the
config is honored. [logic error]
<details>
<summary><b>Severity Level:</b> Major ⚠️</summary>
```mdx
- ❌ Docs build/serve won't fail on broken markdown links.
- ⚠️ Local dev feedback for docs authors is weakened.
- ⚠️ CI link checks may silently pass.
- ⚠️ Issue affects docs developer workflow (yarn start).
```
</details>
```suggestion
onBrokenMarkdownLinks: 'throw',
```
<details>
<summary><b>Steps of Reproduction ✅ </b></summary>
```mdx
1. Open the PR's runtime config at docs/docusaurus.config.ts and locate the
current
setting at lines 171-179. The file currently contains:
- markdown: { mermaid: true, hooks: { onBrokenMarkdownLinks: 'throw' } }
(see
docs/docusaurus.config.ts around lines 171-179).
2. Start the docs dev server locally with the PR changes applied: from the
repo root run
`cd docs && yarn start` (this runs Docusaurus startup which loads
docs/docusaurus.config.ts).
3. Introduce a broken Markdown link in any docs page (for example edit
docs/intro.md and
change a link target to a non-existent doc). Save and let the dev server
rebuild.
4. Observe behavior: because onBrokenMarkdownLinks is nested under
markdown.hooks
(docs/docusaurus.config.ts:176-178) Docusaurus will not pick up the setting
and the broken
link will not trigger the intended 'throw' behavior — the dev server will
continue without
the expected error. This reproduces the issue: the config location prevents
the option
from being honored.
5. Confirm fix by moving the setting to markdown.onBrokenMarkdownLinks
(change as
suggested at docs/docusaurus.config.ts:176 -> onBrokenMarkdownLinks:
'throw',), restart
`yarn start`, re-trigger the broken-link edit and observe the dev build now
throws as
intended.
Note: This reproduction is based on the final config file at
docs/docusaurus.config.ts
(lines 171-179) where the option is currently nested under hooks instead of
placed
directly on the markdown object.
```
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** docs/docusaurus.config.ts
**Line:** 176:178
**Comment:**
*Logic Error: The `onBrokenMarkdownLinks` setting is nested under a
`hooks` object inside `markdown`, which Docusaurus does not read for this
option, so broken Markdown links will not actually cause a throw as intended;
instead, place `onBrokenMarkdownLinks` directly on the `markdown` object so the
config is honored.
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]
