bito-code-review[bot] commented on code in PR #37141:
URL: https://github.com/apache/superset/pull/37141#discussion_r2824560573
##########
superset-frontend/src/pages/Home/index.tsx:
##########
@@ -147,6 +147,7 @@ export const LoadingCards = ({ cover }: LoadingProps) => (
);
function Welcome({ user, addDangerToast }: WelcomeProps) {
+ const { md: isNotMobile } = Grid.useBreakpoint();
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Responsive hook initial render issue</b></div>
<div id="fix">
Grid.useBreakpoint returns an empty object on initial render, making
isNotMobile undefined initially. This causes charts to be hidden on desktop
until the hook updates, creating a flash where content disappears then
reappears. Consider adding a default value or handling the initial state to
avoid this UX issue.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
const { md: isNotMobile = true } = Grid.useBreakpoint();
````
</div>
</details>
</div>
<small><i>Code Review Run #d9a416</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset/static/service-worker.js:
##########
@@ -1,27 +1,1471 @@
-/**
- * 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.
+/*
+ * ATTENTION: An "eval-source-map" devtool has been used.
+ * This devtool is neither made for production nor for readable output files.
+ * It uses "eval()" calls to create a separate source file with attached
SourceMaps in the browser devtools.
+ * If you are trying to read the output file, select a different devtool
(https://webpack.js.org/configuration/devtool/)
+ * or disable the default devtool with "devtool: false".
+ * If you are looking for production-ready output files, see mode:
"production" (https://webpack.js.org/configuration/mode/).
*/
+/******/ (() => { // webpackBootstrap
+/******/ "use strict";
+/******/ var __webpack_modules__ = ({
-// Minimal service worker for PWA file handling support
-self.addEventListener('install', event => {
- event.waitUntil(self.skipWaiting());
-});
+/***/ "./src/service-worker.ts"
+/*!*******************************!*\
+ !*** ./src/service-worker.ts ***!
+ \*******************************/
+() {
-self.addEventListener('activate', event => {
- event.waitUntil(self.clients.claim());
-});
+eval("{/**\n * Licensed to the Apache Software Foundation (ASF) under one\n *
or more contributor license agreements. See the NOTICE file\n * distributed
with this work for additional information\n * regarding copyright ownership.
The ASF licenses this file\n * to you under the Apache License, Version 2.0
(the\n * \"License\"); you may not use this file except in compliance\n * with
the License. You may obtain a copy of the License at\n *\n *
http://www.apache.org/licenses/LICENSE-2.0\n *\n * Unless required by
applicable law or agreed to in writing,\n * software distributed under the
License is distributed on an\n * \"AS IS\" BASIS, WITHOUT WARRANTIES OR
CONDITIONS OF ANY\n * KIND, either express or implied. See the License for
the\n * specific language governing permissions and limitations\n * under the
License.\n */ // Service Worker types (declared locally to avoid polluting
global scope)\nself.addEventListener('install', (event)=>{\n
event.waitUntil(self.skipWaiting())
;\n});\nself.addEventListener('activate', (event)=>{\n
event.waitUntil(self.clients.claim());\n});\n\n//# sourceURL=[module]\n//#
sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJmaWxlIjoiLi9zcmMvc2VydmljZS13b3JrZXIudHMiLCJtYXBwaW5ncyI6IkFBQUE7Ozs7Ozs7Ozs7Ozs7Ozs7O0FBaUJBO0FBWUE7QUFDQTtBQUNBO0FBRUE7QUFDQTtBQUNBO0FBRUEiLCJzb3VyY2VzIjpbIndlYnBhY2s6Ly9zdXBlcnNldC8uL3NyYy9zZXJ2aWNlLXdvcmtlci50cz83ZjU4Il0sInNvdXJjZXNDb250ZW50IjpbIi8qKlxuICogTGljZW5zZWQgdG8gdGhlIEFwYWNoZSBTb2Z0d2FyZSBGb3VuZGF0aW9uIChBU0YpIHVuZGVyIG9uZVxuICogb3IgbW9yZSBjb250cmlidXRvciBsaWNlbnNlIGFncmVlbWVudHMuICBTZWUgdGhlIE5PVElDRSBmaWxlXG4gKiBkaXN0cmlidXRlZCB3aXRoIHRoaXMgd29yayBmb3IgYWRkaXRpb25hbCBpbmZvcm1hdGlvblxuICogcmVnYXJkaW5nIGNvcHlyaWdodCBvd25lcnNoaXAuICBUaGUgQVNGIGxpY2Vuc2VzIHRoaXMgZmlsZVxuICogdG8geW91IHVuZGVyIHRoZSBBcGFjaGUgTGljZW5zZSwgVmVyc2lvbiAyLjAgKHRoZVxuICogXCJMaWNlbnNlXCIpOyB5b3UgbWF5IG5vdCB1c2UgdGhpcyBmaWxlIGV4Y2VwdCBpbiBjb21wbGlhbmNlXG4gKiB3aXRoIHRoZSBMaWNlbnNlLiAgWW91IG1h
eSBvYnRhaW4gYSBjb3B5IG9mIHRoZSBMaWNlbnNlIGF0XG4gKlxuICogICBodHRwOi8vd3d3LmFwYWNoZS5vcmcvbGljZW5zZXMvTElDRU5TRS0yLjBcbiAqXG4gKiBVbmxlc3MgcmVxdWlyZWQgYnkgYXBwbGljYWJsZSBsYXcgb3IgYWdyZWVkIHRvIGluIHdyaXRpbmcsXG4gKiBzb2Z0d2FyZSBkaXN0cmlidXRlZCB1bmRlciB0aGUgTGljZW5zZSBpcyBkaXN0cmlidXRlZCBvbiBhblxuICogXCJBUyBJU1wiIEJBU0lTLCBXSVRIT1VUIFdBUlJBTlRJRVMgT1IgQ09ORElUSU9OUyBPRiBBTllcbiAqIEtJTkQsIGVpdGhlciBleHByZXNzIG9yIGltcGxpZWQuICBTZWUgdGhlIExpY2Vuc2UgZm9yIHRoZVxuICogc3BlY2lmaWMgbGFuZ3VhZ2UgZ292ZXJuaW5nIHBlcm1pc3Npb25zIGFuZCBsaW1pdGF0aW9uc1xuICogdW5kZXIgdGhlIExpY2Vuc2UuXG4gKi9cblxuLy8gU2VydmljZSBXb3JrZXIgdHlwZXMgKGRlY2xhcmVkIGxvY2FsbHkgdG8gYXZvaWQgcG9sbHV0aW5nIGdsb2JhbCBzY29wZSlcbmRlY2xhcmUgY29uc3Qgc2VsZjoge1xuICBza2lwV2FpdGluZygpOiBQcm9taXNlPHZvaWQ+O1xuICBjbGllbnRzOiB7IGNsYWltKCk6IFByb21pc2U8dm9pZD4gfTtcbiAgYWRkRXZlbnRMaXN0ZW5lcihcbiAgICB0eXBlOiAnaW5zdGFsbCcgfCAnYWN0aXZhdGUnLFxuICAgIGxpc3RlbmVyOiAoZXZlbnQ6IHsgd2FpdFVudGlsKHByb21pc2U6IFByb21pc2U8dW5rbm93bj4pOiB2b2lkIH0pID0+IHZvaWQsXG4gICk6IHZva
WQ7XG59O1xuXG5zZWxmLmFkZEV2ZW50TGlzdGVuZXIoJ2luc3RhbGwnLCBldmVudCA9PiB7XG4gIGV2ZW50LndhaXRVbnRpbChzZWxmLnNraXBXYWl0aW5nKCkpO1xufSk7XG5cbnNlbGYuYWRkRXZlbnRMaXN0ZW5lcignYWN0aXZhdGUnLCBldmVudCA9PiB7XG4gIGV2ZW50LndhaXRVbnRpbChzZWxmLmNsaWVudHMuY2xhaW0oKSk7XG59KTtcblxuZXhwb3J0IHt9O1xuIl0sIm5hbWVzIjpbXSwic291cmNlUm9vdCI6IiJ9\n//#
sourceURL=webpack-internal:///./src/service-worker.ts\n\n}");
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Security: Dev artifacts in production service
worker</b></div>
<div id="fix">
The service worker contains eval() calls and inline source maps, which are
development artifacts that create security vulnerabilities in production.
Service workers run with high privileges and should never use eval or expose
source code. Rebuild with production mode to set devtool: false.
</div>
</div>
<small><i>Code Review Run #d9a416</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
##########
superset-frontend/src/components/ListView/utils.ts:
##########
@@ -243,10 +245,19 @@ export function useListViewState({
};
const [viewMode, setViewMode] = useState<ViewModeType>(
- (query.viewMode as ViewModeType) ||
+ // forceViewMode overrides everything (used for mobile)
+ forceViewMode ||
+ (query.viewMode as ViewModeType) ||
(renderCard ? defaultViewMode : 'table'),
);
+ // Update viewMode when forceViewMode changes (e.g., screen resize)
+ useEffect(() => {
+ if (forceViewMode) {
+ setViewMode(forceViewMode);
+ }
+ }, [forceViewMode]);
Review Comment:
<div>
<div id="suggestion">
<div id="issue"><b>Missing reactivity in viewMode state</b></div>
<div id="fix">
The viewMode state is not reactive to changes in query.viewMode, causing the
view mode to not update when the URL query parameter changes. The useEffect
only updates when forceViewMode changes, but ignores changes to query.viewMode,
renderCard, or defaultViewMode.
</div>
<details>
<summary>
<b>Code suggestion</b>
</summary>
<blockquote>Check the AI-generated fix before applying</blockquote>
<div id="code">
````suggestion
// Update viewMode when dependencies change
useEffect(() => {
const computedViewMode = forceViewMode || (query.viewMode as
ViewModeType) || (renderCard ? defaultViewMode : 'table');
setViewMode(computedViewMode);
}, [forceViewMode, query.viewMode, renderCard, defaultViewMode]);
````
</div>
</details>
</div>
<small><i>Code Review Run #d9a416</i></small>
</div>
---
Should Bito avoid suggestions like this for future reviews? (<a
href=https://alpha.bito.ai/home/ai-agents/review-rules>Manage Rules</a>)
- [ ] Yes, avoid them
--
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]
