codeant-ai-for-open-source[bot] commented on code in PR #36816:
URL: https://github.com/apache/superset/pull/36816#discussion_r2643717456
##########
superset/config.py:
##########
@@ -804,9 +811,9 @@ class D3TimeFormat(TypedDict, total=False):
STORE_CACHE_KEYS_IN_METADATA_DB = False
# CORS Options
-ENABLE_CORS = False
+ENABLE_CORS = True
Review Comment:
**Suggestion:** Security risk: `ENABLE_CORS` is enabled by default without
any `CORS_OPTIONS` restricting origins; enabling CORS globally can expose
endpoints to cross-origin requests — revert to disabled by default or require
explicit allowed origins in `CORS_OPTIONS`. [security]
**Severity Level:** Critical 🚨
```suggestion
ENABLE_CORS = False
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
Turning CORS on globally without restricting allowed origins can open APIs
to unwanted cross-origin requests. Reverting to False
or requiring explicit allowed origins in CORS_OPTIONS is a safer default for
most deployments.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/config.py
**Line:** 814:814
**Comment:**
*Security: Security risk: `ENABLE_CORS` is enabled by default without
any `CORS_OPTIONS` restricting origins; enabling CORS globally can expose
endpoints to cross-origin requests — revert to disabled by default or require
explicit allowed origins in `CORS_OPTIONS`.
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>
##########
superset-frontend/src/SqlLab/components/QueryLimitSelect/index.tsx:
##########
@@ -89,7 +92,7 @@ const QueryLimitSelect = ({
const dispatch = useDispatch();
const queryEditor = useQueryEditor(queryEditorId, ['id', 'queryLimit']);
- const queryLimit = queryEditor.queryLimit || defaultQueryLimit;
+ const queryLimit = queryEditor.queryLimit ?? defaultQueryLimit ?? 1000;
Review Comment:
**Suggestion:** Accessing `queryEditor.queryLimit` without optional chaining
can throw if `useQueryEditor` returns null/undefined; use optional chaining so
the nullish coalescing fallback is actually reached instead of causing a
runtime TypeError. [null pointer]
**Severity Level:** Minor ⚠️
```suggestion
const queryLimit = queryEditor?.queryLimit ?? defaultQueryLimit ?? 1000;
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
If useQueryEditor returns undefined/null, accessing queryEditor.queryLimit
will throw. Optional chaining (queryEditor?.queryLimit) prevents a runtime
TypeError and lets the nullish fallback run. This fixes a real runtime crash
scenario.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/SqlLab/components/QueryLimitSelect/index.tsx
**Line:** 95:95
**Comment:**
*Null Pointer: Accessing `queryEditor.queryLimit` without optional
chaining can throw if `useQueryEditor` returns null/undefined; use optional
chaining so the nullish coalescing fallback is actually reached instead of
causing a runtime TypeError.
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>
##########
superset-frontend/src/SqlLab/components/QueryLimitSelect/index.tsx:
##########
@@ -57,11 +57,14 @@ const LimitSelectStyled = styled.span`
`;
function renderQueryLimit(
- maxRow: number,
+ maxRowNumber: number,
setQueryLimit: (limit: number) => void,
) {
const limitDropdown = [];
-
+ let maxRow = maxRowNumber;
+ if (!maxRow) {
Review Comment:
**Suggestion:** The code treats any falsy `maxRow` (including 0) as
"missing" and replaces it with 100000. If `maxRowNumber` is legitimately 0 this
changes behaviour unexpectedly; use a nullish check instead so only
null/undefined triggers the default. Replace the falsy check with a nullish
check (or check for non-positive numbers if desired). [logic error]
**Severity Level:** Minor ⚠️
```suggestion
if (maxRow == null) {
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
Treating any falsy value as missing will convert a legitimate 0 into 100000.
The proposed nullish check (maxRow == null) more precisely covers the intended
case (null/undefined) and avoids silently changing a valid numeric input. The
codebase's types declare maxRow as a number, but runtime values can still be 0,
so this is a sensible correctness improvement.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/SqlLab/components/QueryLimitSelect/index.tsx
**Line:** 65:65
**Comment:**
*Logic Error: The code treats any falsy `maxRow` (including 0) as
"missing" and replaces it with 100000. If `maxRowNumber` is legitimately 0 this
changes behaviour unexpectedly; use a nullish check instead so only
null/undefined triggers the default. Replace the falsy check with a nullish
check (or check for non-positive numbers if desired).
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>
##########
superset-frontend/src/sqlapp/App.tsx:
##########
@@ -0,0 +1,21 @@
+import ToastContainer from 'src/components/MessageToasts/ToastContainer';
+import { GlobalStyles } from 'src/GlobalStyles';
+import SqlLab from 'src/pages/SqlLab';
+import setupApp from 'src/setup/setupApp';
+import setupExtensions from 'src/setup/setupExtensions';
+import setupPlugins from 'src/setup/setupPlugins';
+import { RootContextProviders } from 'src/views/RootContextProviders';
+import { BrowserRouter as Router } from 'react-router-dom';
+setupApp();
+setupPlugins();
+setupExtensions();
Review Comment:
**Suggestion:** Missing await on potential async initializers: the file
calls `setupApp()`, `setupPlugins()`, and `setupExtensions()` directly without
awaiting; if any of these return a Promise, initialization may not complete
before the app renders, causing unpredictable runtime behavior. Wrap and await
them (for example via an async IIFE) so the initialization sequence is
preserved. [possible bug]
**Severity Level:** Critical 🚨
```suggestion
(async () => {
await setupApp();
await setupPlugins();
await setupExtensions();
})();
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
If any of those functions return Promises, not awaiting them can cause race
conditions where parts of the app render before initialization completes.
Wrapping and awaiting them (even in an IIFE) fixes that specific problem. Note
this doesn't address the module-level side-effect / SSR problem — it only
ensures ordering — so it's a valid, focused fix.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/sqlapp/App.tsx
**Line:** 9:11
**Comment:**
*Possible Bug: Missing await on potential async initializers: the file
calls `setupApp()`, `setupPlugins()`, and `setupExtensions()` directly without
awaiting; if any of these return a Promise, initialization may not complete
before the app renders, causing unpredictable runtime behavior. Wrap and await
them (for example via an async IIFE) so the initialization sequence is
preserved.
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>
##########
superset/config.py:
##########
@@ -1046,7 +1053,7 @@ class CeleryConfig: # pylint:
disable=too-few-public-methods
# override anything set within the app
DEFAULT_HTTP_HEADERS: dict[str, Any] = {}
OVERRIDE_HTTP_HEADERS: dict[str, Any] = {}
-HTTP_HEADERS: dict[str, Any] = {}
+HTTP_HEADERS: dict[str, Any] = {'X-Frame-Options': 'ALLOWALL'}
Review Comment:
**Suggestion:** Security issue: `HTTP_HEADERS` sets `X-Frame-Options` to the
non-standard value `ALLOWALL`, which is invalid and weakens clickjacking
protection; use a standard safe value such as `SAMEORIGIN` (or remove the
header and rely on CSP/frame-ancestors). [security]
**Severity Level:** Critical 🚨
```suggestion
HTTP_HEADERS: dict[str, Any] = {'X-Frame-Options': 'SAMEORIGIN'}
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
'ALLOWALL' is not a standard X-Frame-Options value and effectively disables
clickjacking protection by allowing framing.
Using a standard value like 'SAMEORIGIN' (or removing the header and relying
on a proper CSP/frame-ancestors) restores expected protection.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/config.py
**Line:** 1056:1056
**Comment:**
*Security: Security issue: `HTTP_HEADERS` sets `X-Frame-Options` to the
non-standard value `ALLOWALL`, which is invalid and weakens clickjacking
protection; use a standard safe value such as `SAMEORIGIN` (or remove the
header and rely on CSP/frame-ancestors).
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>
##########
superset-frontend/src/sqlapp/index.tsx:
##########
@@ -0,0 +1,6 @@
+import ReactDOM from 'react-dom';
+import { App } from './App';
+
+const el = document.getElementById('app');
+console.log('heeofre', el);
+ReactDOM.render(<App />, el);
Review Comment:
**Suggestion:** Missing null-check before calling ReactDOM.render: if
`document.getElementById('app')` returns null (element not present), React will
throw "Target container is not a DOM element." Add a guard and fail-fast or
return early before calling render. [possible bug]
**Severity Level:** Critical 🚨
```suggestion
if (!el) {
throw new Error("Root element with id 'app' not found");
}
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The current code will throw "Target container is not a DOM element." if
getElementById returns null.
Adding a guard (or returning early) is a small, correct hardening that
prevents a runtime crash and gives a clearer error.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/src/sqlapp/index.tsx
**Line:** 6:6
**Comment:**
*Possible Bug: Missing null-check before calling ReactDOM.render: if
`document.getElementById('app')` returns null (element not present), React will
throw "Target container is not a DOM element." Add a guard and fail-fast or
return early before calling render.
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>
##########
superset-jupyter/index.html:
##########
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <meta http-equiv="content-type" content="text/html; charset=UTF8">
Review Comment:
**Suggestion:** Incorrect charset declaration: `charset=UTF8` is
non-standard; use the canonical UTF-8 declaration (for example `<meta
charset="utf-8">`) so browsers correctly detect character encoding. [possible
bug]
**Severity Level:** Critical 🚨
```suggestion
<meta charset="utf-8">
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The canonical declaration is <meta charset=\"utf-8\">; the current
http-equiv usage with charset=UTF8 is non-standard and could be misinterpreted.
Changing it to the standard form fixes potential charset detection issues.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-jupyter/index.html
**Line:** 4:4
**Comment:**
*Possible Bug: Incorrect charset declaration: `charset=UTF8` is
non-standard; use the canonical UTF-8 declaration (for example `<meta
charset="utf-8">`) so browsers correctly detect character encoding.
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>
##########
superset-jupyter/index.html:
##########
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <meta http-equiv="content-type" content="text/html; charset=UTF8">
+
+
+
+ <!-- <link rel="stylesheet" type="text/css"
href="/static/assets/theme.entry.css" /> -->
+
+
+
+ <!-- <link rel="stylesheet" type="text/css"
href="/static/assets/spa.entry.css" />
+ -->
+>
+
+ <script src="/static/assets/theme.entry.js" async nonce=""></script>
+
+
+
+ <head/>
Review Comment:
**Suggestion:** Malformed head closing tag: using a self-closing `<head/>`
instead of the correct `</head>` will produce invalid HTML structure which may
cause browsers to misplace elements (scripts/styles) and break page load order;
replace with a proper closing tag. [logic error]
**Severity Level:** Minor ⚠️
```suggestion
</head>
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
<head/> is not a valid HTML head closing token — it should be </head>.
Replacing it fixes the document structure so the head content is correctly
scoped; this is a real, verifiable correctness bug in the final file.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-jupyter/index.html
**Line:** 20:20
**Comment:**
*Logic Error: Malformed head closing tag: using a self-closing
`<head/>` instead of the correct `</head>` will produce invalid HTML structure
which may cause browsers to misplace elements (scripts/styles) and break page
load order; replace with a proper closing tag.
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>
##########
superset/config.py:
##########
@@ -804,9 +811,9 @@ class D3TimeFormat(TypedDict, total=False):
STORE_CACHE_KEYS_IN_METADATA_DB = False
# CORS Options
-ENABLE_CORS = False
+ENABLE_CORS = True
CORS_OPTIONS: dict[Any, Any] = {}
-
+PUBLIC_ROLE_LIKE_GAMMA = True
Review Comment:
**Suggestion:** Logic bug: a new boolean `PUBLIC_ROLE_LIKE_GAMMA` is defined
but the application expects `PUBLIC_ROLE_LIKE` to be the role name (string).
This new setting will be ignored and will not grant the Public role the
intended permissions; replace it with setting `PUBLIC_ROLE_LIKE` to the builtin
role name (e.g., "Gamma"). [logic error]
**Severity Level:** Minor ⚠️
```suggestion
PUBLIC_ROLE_LIKE = "Gamma"
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The codebase already uses PUBLIC_ROLE_LIKE (string or None) to grant the
Public role the same permissions as a built-in role.
Adding PUBLIC_ROLE_LIKE_GAMMA = True does nothing — it's a new boolean that
the rest of the app won't read. Replacing it with
PUBLIC_ROLE_LIKE = "Gamma" fixes the logic so the Public role actually
inherits Gamma permissions.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset/config.py
**Line:** 816:816
**Comment:**
*Logic Error: Logic bug: a new boolean `PUBLIC_ROLE_LIKE_GAMMA` is
defined but the application expects `PUBLIC_ROLE_LIKE` to be the role name
(string). This new setting will be ignored and will not grant the Public role
the intended permissions; replace it with setting `PUBLIC_ROLE_LIKE` to the
builtin role name (e.g., "Gamma").
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>
##########
superset-frontend/webpack.jupyter.js:
##########
@@ -0,0 +1,601 @@
+/* eslint-disable no-console */
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+const fs = require('fs');
+const path = require('path');
+const webpack = require('webpack');
+const { BundleAnalyzerPlugin } = require('webpack-bundle-analyzer');
+const CopyPlugin = require('copy-webpack-plugin');
+const HtmlWebpackPlugin = require('html-webpack-plugin');
+const MiniCssExtractPlugin = require('mini-css-extract-plugin');
+const MomentLocalesPlugin = require('moment-locales-webpack-plugin');
+const CssMinimizerPlugin = require('css-minimizer-webpack-plugin');
+const SpeedMeasurePlugin = require('speed-measure-webpack-plugin');
+const {
+ WebpackManifestPlugin,
+ getCompilerHooks,
+} = require('webpack-manifest-plugin');
+const ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin');
+const parsedArgs = require('yargs').argv;
+const Visualizer = require('webpack-visualizer-plugin2');
+const getProxyConfig = require('./webpack.proxy-config');
+const packageConfig = require('./package');
+
+// input dir
+const APP_DIR = path.resolve(__dirname, './');
+// output dir
+const BUILD_DIR = path.resolve(__dirname, '../superset-jupyter/static/assets');
+const ROOT_DIR = path.resolve(__dirname, '..');
+const TRANSLATIONS_DIR = path.resolve(__dirname, '../superset/translations');
+
+const getAvailableTranslationCodes = () => {
+ if (process.env.BUILD_TRANSLATIONS === 'true') {
+ const LOCALE_CODE_MAPPING = {
+ zh: 'zh-cn',
+ };
+ const files = fs.readdirSync(TRANSLATIONS_DIR);
+ return files
+ .filter(file =>
+ fs.statSync(path.join(TRANSLATIONS_DIR, file)).isDirectory(),
+ )
+ .filter(dirName => !dirName.startsWith('__'))
+ .map(dirName => dirName.replace('_', '-'))
+ .map(dirName => LOCALE_CODE_MAPPING[dirName] || dirName);
+ }
+ // Indicates to the MomentLocalesPlugin that we only want to keep 'en'.
+ return [];
+};
+
+const {
+ mode = 'development',
+ devserverPort = 9000,
+ measure = false,
+ nameChunks = false,
+} = parsedArgs;
+const isDevMode = mode !== 'production';
+const isDevServer = process.argv[1].includes('webpack-dev-server');
+const ASSET_BASE_URL = process.env.ASSET_BASE_URL || '';
+
+const output = {
+ path: BUILD_DIR,
+ publicPath: `${ASSET_BASE_URL}/static/assets/`,
+};
+
+if (isDevMode) {
+ output.filename = '[name].entry.js';
+ output.chunkFilename = '[name].[contenthash:8].chunk.js';
+} else if (nameChunks) {
+ output.filename = '[name].[chunkhash].entry.js';
+ output.chunkFilename = '[name].[chunkhash].chunk.js';
+} else {
+ output.filename = '[name].[chunkhash].entry.js';
+ output.chunkFilename = '[chunkhash].chunk.js';
+}
+
+if (!isDevMode) {
+ output.clean = true;
+}
+
+const plugins = [
+ new webpack.ProvidePlugin({
+ process: 'process/browser.js',
+ ...(isDevMode ? { Buffer: ['buffer', 'Buffer'] } : {}), // Fix
legacy-plugin-chart-paired-t-test broken Story
+ }),
+
+ // creates a manifest.json mapping of name to hashed output used in template
files
+ new WebpackManifestPlugin({
+ publicPath: output.publicPath,
+ seed: { app: 'superset' },
+ // This enables us to include all relevant files for an entry
+ generate: (seed, files, entrypoints) => {
+ // Each entrypoint's chunk files in the format of
+ // {
+ // entry: {
+ // css: [],
+ // js: []
+ // }
+ // }
+ const entryFiles = {};
+ Object.entries(entrypoints).forEach(([entry, chunks]) => {
+ entryFiles[entry] = {
+ css: chunks
+ .filter(x => x.endsWith('.css'))
+ .map(x => `${output.publicPath}${x}`),
+ js: chunks
+ .filter(x => x.endsWith('.js') && x.match(/(?<!hot-update).js$/))
Review Comment:
**Suggestion:** The manifest generation filters JS chunks with a regex using
a negative lookbehind and an unescaped dot (/.js$/) which is both fragile
(lookbehind may not be supported on all Node versions) and incorrect (the dot
should be escaped); replace the regex with a simple `!x.includes('hot-update')`
check to reliably exclude hot-update files. [possible bug]
**Severity Level:** Critical 🚨
```suggestion
.filter(x => x.endsWith('.js') && !x.includes('hot-update'))
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The regex uses a negative lookbehind and an unescaped dot — both fragile:
lookbehind may not be supported on some Node versions and the pattern doesn't
clearly express intent. Replacing with a simple !x.includes('hot-update') is
clearer, more compatible and fixes the logic of excluding hot-update files.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/webpack.jupyter.js
**Line:** 121:121
**Comment:**
*Possible Bug: The manifest generation filters JS chunks with a regex
using a negative lookbehind and an unescaped dot (/.js$/) which is both fragile
(lookbehind may not be supported on all Node versions) and incorrect (the dot
should be escaped); replace the regex with a simple `!x.includes('hot-update')`
check to reliably exclude hot-update files.
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>
##########
superset-jupyter/index.html:
##########
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <meta http-equiv="content-type" content="text/html; charset=UTF8">
+
+
+
+ <!-- <link rel="stylesheet" type="text/css"
href="/static/assets/theme.entry.css" /> -->
+
+
+
+ <!-- <link rel="stylesheet" type="text/css"
href="/static/assets/spa.entry.css" />
+ -->
+>
Review Comment:
**Suggestion:** Stray standalone ">" character present in the document
creates an unexpected text node and indicates leftover/garbage markup; remove
this line to avoid rendering artifacts and potential parsing ambiguity.
[possible bug]
**Severity Level:** Critical 🚨
```suggestion
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
A stray standalone ">" creates an unexpected text node and indicates
leftover/garbage markup; removing it prevents parsing/rendering artifacts. This
is a simple, correct clean-up tied to the diff.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-jupyter/index.html
**Line:** 14:14
**Comment:**
*Possible Bug: Stray standalone ">" character present in the document
creates an unexpected text node and indicates leftover/garbage markup; remove
this line to avoid rendering artifacts and potential parsing ambiguity.
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>
##########
superset-jupyter/index.html:
##########
@@ -0,0 +1,27 @@
+<!DOCTYPE html>
+<html>
+ <head>
+ <meta http-equiv="content-type" content="text/html; charset=UTF8">
+
+
+
+ <!-- <link rel="stylesheet" type="text/css"
href="/static/assets/theme.entry.css" /> -->
+
+
+
+ <!-- <link rel="stylesheet" type="text/css"
href="/static/assets/spa.entry.css" />
+ -->
+>
+
+ <script src="/static/assets/theme.entry.js" async nonce=""></script>
+
+
+
+ <head/>
+ <body>
+ <div id="app" style="width: 100vw;height:100vh"></div>
+ </body>
+ <script src="./static/assets/thumbnail.entry.js"></script>
+ <script src="./static/assets/sql.entry.js"></script>
Review Comment:
**Suggestion:** Inconsistent/relative script paths: the bottom scripts use
relative `./static/...` while the top script uses `/static/...`; relative paths
can break when the page is served from a nested URL — make these absolute or
consistent (e.g. `/static/...`) to avoid 404s. [logic error]
**Severity Level:** Minor ⚠️
```suggestion
<script src="/static/assets/thumbnail.entry.js"></script>
<script src="/static/assets/sql.entry.js"></script>
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The top script uses an absolute /static path while the bottom ones use
./static — that inconsistency can cause 404s when the page is served from
nested paths. Making them consistent (absolute) prevents a class of
deployment-dependent failures.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-jupyter/index.html
**Line:** 24:25
**Comment:**
*Logic Error: Inconsistent/relative script paths: the bottom scripts
use relative `./static/...` while the top script uses `/static/...`; relative
paths can break when the page is served from a nested URL — make these absolute
or consistent (e.g. `/static/...`) to avoid 404s.
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>
##########
superset-frontend/webpack.jupyter.js:
##########
@@ -0,0 +1,601 @@
+/* eslint-disable no-console */
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+const fs = require('fs');
+const path = require('path');
+const webpack = require('webpack');
+const { BundleAnalyzerPlugin } = require('webpack-bundle-analyzer');
+const CopyPlugin = require('copy-webpack-plugin');
+const HtmlWebpackPlugin = require('html-webpack-plugin');
+const MiniCssExtractPlugin = require('mini-css-extract-plugin');
+const MomentLocalesPlugin = require('moment-locales-webpack-plugin');
+const CssMinimizerPlugin = require('css-minimizer-webpack-plugin');
+const SpeedMeasurePlugin = require('speed-measure-webpack-plugin');
+const {
+ WebpackManifestPlugin,
+ getCompilerHooks,
+} = require('webpack-manifest-plugin');
+const ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin');
+const parsedArgs = require('yargs').argv;
+const Visualizer = require('webpack-visualizer-plugin2');
+const getProxyConfig = require('./webpack.proxy-config');
+const packageConfig = require('./package');
+
+// input dir
+const APP_DIR = path.resolve(__dirname, './');
+// output dir
+const BUILD_DIR = path.resolve(__dirname, '../superset-jupyter/static/assets');
+const ROOT_DIR = path.resolve(__dirname, '..');
+const TRANSLATIONS_DIR = path.resolve(__dirname, '../superset/translations');
+
+const getAvailableTranslationCodes = () => {
+ if (process.env.BUILD_TRANSLATIONS === 'true') {
+ const LOCALE_CODE_MAPPING = {
+ zh: 'zh-cn',
+ };
+ const files = fs.readdirSync(TRANSLATIONS_DIR);
+ return files
+ .filter(file =>
+ fs.statSync(path.join(TRANSLATIONS_DIR, file)).isDirectory(),
+ )
+ .filter(dirName => !dirName.startsWith('__'))
+ .map(dirName => dirName.replace('_', '-'))
Review Comment:
**Suggestion:** The translation directory name replacement uses
`dirName.replace('_', '-')`, which only replaces the first underscore; use a
global replace to convert all underscores to hyphens so locale codes like
`pt_br` become `pt-br` consistently. [logic error]
**Severity Level:** Minor ⚠️
```suggestion
.map(dirName => dirName.replace(/_/g, '-'))
```
<details>
<summary><b>Why it matters? ⭐ </b></summary>
The current code only replaces the first underscore, so multi-part locale
names (e.g., pt_br) become pt-br only for the first occurrence if more exist.
Switching to a global replace (/g) fixes a real normalization bug and is a
safe, minimal change.
</details>
<details>
<summary><b>Prompt for AI Agent 🤖 </b></summary>
```mdx
This is a comment left during a code review.
**Path:** superset-frontend/webpack.jupyter.js
**Line:** 58:58
**Comment:**
*Logic Error: The translation directory name replacement uses
`dirName.replace('_', '-')`, which only replaces the first underscore; use a
global replace to convert all underscores to hyphens so locale codes like
`pt_br` become `pt-br` consistently.
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]
