korbit-ai[bot] commented on code in PR #35506: URL: https://github.com/apache/superset/pull/35506#discussion_r2449357565
########## superset-frontend/plugins/plugin-chart-ag-grid-table/src/renderers/NumericCellRenderer.tsx: ########## @@ -150,6 +152,7 @@ export const NumericCellRenderer = ( } = params; const isDarkTheme = useIsDark(); + const theme = useTheme(); Review Comment: ### Unnecessary theme hook execution <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? The useTheme hook is called on every render of NumericCellRenderer, but the theme object is only used in the cellBackground function which may not execute on every render. ###### Why this matters This creates unnecessary overhead as the theme hook will execute and potentially trigger re-renders even when the theme values aren't needed, such as when valueRange is falsy or when colorPositiveNegative is true. ###### Suggested change ∙ *Feature Preview* Move the useTheme call inside the cellBackground function or conditionally call it only when needed: ```typescript const theme = !colorPositiveNegative ? useTheme() : null; ``` Or refactor cellBackground to accept a theme getter function instead of the theme object directly. ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/9e6e1cab-d8e6-4bd8-8bf2-98f7928f8a1a/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/9e6e1cab-d8e6-4bd8-8bf2-98f7928f8a1a?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/9e6e1cab-d8e6-4bd8-8bf2-98f7928f8a1a?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/9e6e1cab-d8e6-4bd8-8bf2-98f7928f8a1a?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/9e6e1cab-d8e6-4bd8-8bf2-98f7928f8a1a) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:3c4775dd-569a-43bf-957e-df96c4b27d8f --> [](3c4775dd-569a-43bf-957e-df96c4b27d8f) ########## superset-frontend/scripts/oxlint-metrics-uploader.js: ########## @@ -0,0 +1,245 @@ +/** + * 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 { execSync } = require('child_process'); +const { google } = require('googleapis'); + +const { SPREADSHEET_ID } = process.env; +const SERVICE_ACCOUNT_KEY = JSON.parse(process.env.SERVICE_ACCOUNT_KEY || '{}'); + +// Only set up Google Sheets if we have credentials +let sheets; +if (SERVICE_ACCOUNT_KEY.client_email) { + const auth = new google.auth.GoogleAuth({ + credentials: SERVICE_ACCOUNT_KEY, + scopes: ['https://www.googleapis.com/auth/spreadsheets'], + }); + sheets = google.sheets({ version: 'v4', auth }); +} + +const DATETIME = new Date().toISOString().replace(/T/, ' ').replace(/\..+/, ''); + +async function writeToGoogleSheet(data, range, headers, append = false) { + if (!sheets) { + console.log('No Google Sheets credentials, skipping upload'); + return; + } + + const request = { + spreadsheetId: SPREADSHEET_ID, + range, + valueInputOption: 'USER_ENTERED', + resource: { values: append ? data : [headers, ...data] }, + }; + + const method = append ? 'append' : 'update'; + await sheets.spreadsheets.values[method](request); +} + +// Run OXC and get JSON output +async function runOxlintAndProcess() { + const enrichedRules = { + 'react-prefer-function-component/react-prefer-function-component': { + description: 'We prefer function components to class-based components', + }, + 'react/jsx-filename-extension': { + description: + 'We prefer Typescript - all JSX files should be converted to TSX', + }, + 'react/forbid-component-props': { + description: + 'We prefer Emotion for styling rather than `className` or `style` props', + }, + 'no-restricted-imports': { + description: + "This rule catches several things that shouldn't be used anymore. LESS, antD, etc. See individual occurrence messages for details", + }, + 'no-console': { + description: + "We don't want a bunch of console noise, but you can use the `logger` from `@superset-ui/core` when there's a reason to.", + }, + }; + + try { + // Run OXC with JSON format + console.log('Running OXC linter...'); + const oxlintOutput = execSync('npx oxlint --format json', { + encoding: 'utf8', + maxBuffer: 50 * 1024 * 1024, // 50MB buffer for large outputs + stdio: ['pipe', 'pipe', 'ignore'], // Ignore stderr to avoid error output + }); + + const results = JSON.parse(oxlintOutput); + + // Process OXC JSON output + const metricsByRule = {}; + let occurrencesData = []; + + // OXC JSON format has diagnostics array + if (results.diagnostics && Array.isArray(results.diagnostics)) { + results.diagnostics.forEach(diagnostic => { + // Extract rule ID from code like "eslint(no-unused-vars)" or "eslint-plugin-unicorn(no-new-array)" + const codeMatch = diagnostic.code?.match( + /^(?:eslint(?:-plugin-(\w+))?\()([^)]+)\)$/, + ); + let ruleId = diagnostic.code || 'unknown'; + + if (codeMatch) { + const plugin = codeMatch[1]; + const rule = codeMatch[2]; + ruleId = plugin ? `${plugin}/${rule}` : rule; + } + + const file = diagnostic.filename || 'unknown'; + const line = diagnostic.labels?.[0]?.span?.line || 0; + const column = diagnostic.labels?.[0]?.span?.column || 0; Review Comment: ### Invalid default line/column values <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? Line and column numbers are defaulted to 0, which is not a valid line/column position in most editors and could confuse developers trying to locate issues. ###### Why this matters Developers will be unable to navigate to the actual location of lint issues when the line/column data is missing, reducing the utility of the uploaded metrics. ###### Suggested change ∙ *Feature Preview* Use 1-based indexing as the default, which is standard for most editors: ```javascript const line = diagnostic.labels?.[0]?.span?.line || 1; const column = diagnostic.labels?.[0]?.span?.column || 1; ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/5d292a8a-66e1-4a01-a9ca-b4e19c286ae0/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/5d292a8a-66e1-4a01-a9ca-b4e19c286ae0?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/5d292a8a-66e1-4a01-a9ca-b4e19c286ae0?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/5d292a8a-66e1-4a01-a9ca-b4e19c286ae0?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/5d292a8a-66e1-4a01-a9ca-b4e19c286ae0) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:68e388eb-3ea1-4312-9200-7bbd8ea7948d --> [](68e388eb-3ea1-4312-9200-7bbd8ea7948d) ########## superset-frontend/packages/superset-ui-demo/storybook/stories/superset-ui-theme/Theme.stories.tsx: ########## @@ -66,15 +59,17 @@ const AntDFunctionalColors = () => { <strong>{type}</strong> </td> {variants.map(variant => { - const color = themeObject.getColorVariants(type)[variant]; + // Map to actual theme token names + const tokenName = `color${type.charAt(0).toUpperCase() + type.slice(1)}${variant.charAt(0).toUpperCase() + variant.slice(1)}`; Review Comment: ### String Manipulation Logic in Render Method <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? String manipulation logic for creating token names is embedded directly in the render method and repeated for each iteration. ###### Why this matters This string manipulation logic clutters the rendering code and could be reused elsewhere. It violates the Single Responsibility Principle by mixing string manipulation with rendering logic. ###### Suggested change ∙ *Feature Preview* Extract the token name generation into a utility function: ```typescript const generateTokenName = (type: string, variant: string) => `color${type.charAt(0).toUpperCase() + type.slice(1)}${variant.charAt(0).toUpperCase() + variant.slice(1)}`; // Then in the component: const tokenName = generateTokenName(type, variant); ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/eeb16baa-ce2c-4947-90a3-77776b27a006/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/eeb16baa-ce2c-4947-90a3-77776b27a006?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/eeb16baa-ce2c-4947-90a3-77776b27a006?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/eeb16baa-ce2c-4947-90a3-77776b27a006?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/eeb16baa-ce2c-4947-90a3-77776b27a006) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:2fda60f3-e88f-4779-8d08-d6d02d8f1702 --> [](2fda60f3-e88f-4779-8d08-d6d02d8f1702) ########## superset-frontend/scripts/oxlint-metrics-uploader.js: ########## @@ -0,0 +1,245 @@ +/** + * 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 { execSync } = require('child_process'); +const { google } = require('googleapis'); + +const { SPREADSHEET_ID } = process.env; +const SERVICE_ACCOUNT_KEY = JSON.parse(process.env.SERVICE_ACCOUNT_KEY || '{}'); + +// Only set up Google Sheets if we have credentials +let sheets; +if (SERVICE_ACCOUNT_KEY.client_email) { + const auth = new google.auth.GoogleAuth({ + credentials: SERVICE_ACCOUNT_KEY, + scopes: ['https://www.googleapis.com/auth/spreadsheets'], + }); + sheets = google.sheets({ version: 'v4', auth }); +} Review Comment: ### Missing abstraction for data storage <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? Google Sheets integration is tightly coupled with the main script logic, making it difficult to swap out different storage backends. ###### Why this matters Direct dependency on Google Sheets makes the code less flexible and harder to test. It violates the Dependency Inversion Principle. ###### Suggested change ∙ *Feature Preview* Create an abstract storage interface: ```javascript class MetricsStorage { async save(data) {} } class GoogleSheetsStorage extends MetricsStorage { async save(data) { // Google Sheets specific implementation } } ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/47d350ea-37b2-41c7-9c31-4f31f24bdf09/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/47d350ea-37b2-41c7-9c31-4f31f24bdf09?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/47d350ea-37b2-41c7-9c31-4f31f24bdf09?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/47d350ea-37b2-41c7-9c31-4f31f24bdf09?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/47d350ea-37b2-41c7-9c31-4f31f24bdf09) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:7a3c3b64-371d-435e-a6af-d5a68c0f6797 --> [](7a3c3b64-371d-435e-a6af-d5a68c0f6797) ########## superset-frontend/packages/superset-ui-demo/storybook/stories/superset-ui-theme/Theme.stories.tsx: ########## @@ -66,15 +59,17 @@ const AntDFunctionalColors = () => { <strong>{type}</strong> </td> {variants.map(variant => { - const color = themeObject.getColorVariants(type)[variant]; + // Map to actual theme token names + const tokenName = `color${type.charAt(0).toUpperCase() + type.slice(1)}${variant.charAt(0).toUpperCase() + variant.slice(1)}`; + const color = (supersetTheme as any)[tokenName]; return ( <td key={variant} style={{ border: '1px solid #ddd', padding: '8px', backgroundColor: color || 'transparent', - color: `color${type}${variant}`, + color: color === 'transparent' ? 'black' : undefined, Review Comment: ### Inconsistent transparent color handling logic <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? The logic checks if color equals 'transparent' string but sets backgroundColor to 'transparent' when color is falsy, creating inconsistent behavior between the two style properties. ###### Why this matters When color is undefined/null/empty, backgroundColor becomes 'transparent' but the text color condition fails since undefined !== 'transparent', potentially resulting in invisible text on transparent backgrounds. ###### Suggested change ∙ *Feature Preview* Make the logic consistent by checking for the same condition: ```typescript const resolvedColor = color || 'transparent'; return ( <td key={variant} style={{ border: '1px solid #ddd', padding: '8px', backgroundColor: resolvedColor, color: resolvedColor === 'transparent' ? 'black' : undefined, }} > {color ? <code>{color}</code> : '-'} </td> ); ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/873b17a9-9df7-4c45-8508-93ba93e190fa/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/873b17a9-9df7-4c45-8508-93ba93e190fa?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/873b17a9-9df7-4c45-8508-93ba93e190fa?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/873b17a9-9df7-4c45-8508-93ba93e190fa?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/873b17a9-9df7-4c45-8508-93ba93e190fa) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:b417ab94-6a0d-4c98-aeda-6ddf00bb539e --> [](b417ab94-6a0d-4c98-aeda-6ddf00bb539e) ########## superset-frontend/packages/superset-ui-demo/storybook/stories/plugins/plugin-chart-table/testData.ts: ########## @@ -29,7 +26,7 @@ import { // eslint-disable-next-line import/extensions import birthNamesJson from './birthNames.json'; -export const birthNames = birthNamesJson as TableChartProps; +export const birthNames = birthNamesJson as unknown as TableChartProps; Review Comment: ### Unsafe type assertion masks data/type mismatches <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? Unsafe type assertion masks potential type mismatches between birthNamesJson and TableChartProps. ###### Why this matters Casting via 'as unknown as TableChartProps' bypasses compile-time type validation, which can allow invalid or unexpected data to flow into the component at runtime, leading to runtime errors or incorrect UI behavior. ###### Suggested change ∙ *Feature Preview* Use a direct, proper type assertion or validate the JSON shape before casting. For example: ```ts export const birthNames = birthNamesJson as TableChartProps; ``` If safety is a concern, add a runtime validation step to ensure birthNamesJson conforms to TableChartProps before using it. ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d8f3ae80-bb73-4515-ac05-9c0b0f942d80/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d8f3ae80-bb73-4515-ac05-9c0b0f942d80?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d8f3ae80-bb73-4515-ac05-9c0b0f942d80?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d8f3ae80-bb73-4515-ac05-9c0b0f942d80?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d8f3ae80-bb73-4515-ac05-9c0b0f942d80) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:c2bcdfac-c9f0-41e3-a98a-a08f757e8098 --> [](c2bcdfac-c9f0-41e3-a98a-a08f757e8098) ########## superset-frontend/scripts/oxlint-metrics-uploader.js: ########## @@ -0,0 +1,245 @@ +/** + * 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 { execSync } = require('child_process'); +const { google } = require('googleapis'); + +const { SPREADSHEET_ID } = process.env; +const SERVICE_ACCOUNT_KEY = JSON.parse(process.env.SERVICE_ACCOUNT_KEY || '{}'); + +// Only set up Google Sheets if we have credentials +let sheets; +if (SERVICE_ACCOUNT_KEY.client_email) { + const auth = new google.auth.GoogleAuth({ + credentials: SERVICE_ACCOUNT_KEY, + scopes: ['https://www.googleapis.com/auth/spreadsheets'], + }); + sheets = google.sheets({ version: 'v4', auth }); +} + +const DATETIME = new Date().toISOString().replace(/T/, ' ').replace(/\..+/, ''); + +async function writeToGoogleSheet(data, range, headers, append = false) { + if (!sheets) { + console.log('No Google Sheets credentials, skipping upload'); + return; + } + + const request = { + spreadsheetId: SPREADSHEET_ID, + range, + valueInputOption: 'USER_ENTERED', + resource: { values: append ? data : [headers, ...data] }, + }; + + const method = append ? 'append' : 'update'; + await sheets.spreadsheets.values[method](request); +} + +// Run OXC and get JSON output +async function runOxlintAndProcess() { + const enrichedRules = { + 'react-prefer-function-component/react-prefer-function-component': { + description: 'We prefer function components to class-based components', + }, + 'react/jsx-filename-extension': { + description: + 'We prefer Typescript - all JSX files should be converted to TSX', + }, + 'react/forbid-component-props': { + description: + 'We prefer Emotion for styling rather than `className` or `style` props', + }, + 'no-restricted-imports': { + description: + "This rule catches several things that shouldn't be used anymore. LESS, antD, etc. See individual occurrence messages for details", + }, + 'no-console': { + description: + "We don't want a bunch of console noise, but you can use the `logger` from `@superset-ui/core` when there's a reason to.", + }, + }; Review Comment: ### Hardcoded configuration needs externalization <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? Rule configurations are hardcoded within the script rather than externalized into a configuration file. ###### Why this matters Embedding configuration in code makes it harder to maintain and update rules without modifying the script itself. This violates separation of concerns. ###### Suggested change ∙ *Feature Preview* Move rules to a separate configuration file: ```javascript // rules-config.js module.exports = { enrichedRules: { // rule definitions } }; // main script const { enrichedRules } = require('./rules-config'); ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d25186ca-db5a-4663-aee3-a066e164504e/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d25186ca-db5a-4663-aee3-a066e164504e?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d25186ca-db5a-4663-aee3-a066e164504e?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d25186ca-db5a-4663-aee3-a066e164504e?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/d25186ca-db5a-4663-aee3-a066e164504e) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:d45e8b7f-2a25-4d8e-9150-dd4fca9d4405 --> [](d45e8b7f-2a25-4d8e-9150-dd4fca9d4405) ########## superset-frontend/scripts/oxlint-metrics-uploader.js: ########## @@ -0,0 +1,245 @@ +/** + * 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 { execSync } = require('child_process'); +const { google } = require('googleapis'); + +const { SPREADSHEET_ID } = process.env; +const SERVICE_ACCOUNT_KEY = JSON.parse(process.env.SERVICE_ACCOUNT_KEY || '{}'); + +// Only set up Google Sheets if we have credentials +let sheets; +if (SERVICE_ACCOUNT_KEY.client_email) { + const auth = new google.auth.GoogleAuth({ + credentials: SERVICE_ACCOUNT_KEY, + scopes: ['https://www.googleapis.com/auth/spreadsheets'], + }); + sheets = google.sheets({ version: 'v4', auth }); +} + +const DATETIME = new Date().toISOString().replace(/T/, ' ').replace(/\..+/, ''); + +async function writeToGoogleSheet(data, range, headers, append = false) { + if (!sheets) { + console.log('No Google Sheets credentials, skipping upload'); + return; + } + + const request = { + spreadsheetId: SPREADSHEET_ID, + range, + valueInputOption: 'USER_ENTERED', + resource: { values: append ? data : [headers, ...data] }, + }; + + const method = append ? 'append' : 'update'; + await sheets.spreadsheets.values[method](request); +} + +// Run OXC and get JSON output +async function runOxlintAndProcess() { + const enrichedRules = { + 'react-prefer-function-component/react-prefer-function-component': { + description: 'We prefer function components to class-based components', + }, + 'react/jsx-filename-extension': { + description: + 'We prefer Typescript - all JSX files should be converted to TSX', + }, + 'react/forbid-component-props': { + description: + 'We prefer Emotion for styling rather than `className` or `style` props', + }, + 'no-restricted-imports': { + description: + "This rule catches several things that shouldn't be used anymore. LESS, antD, etc. See individual occurrence messages for details", + }, + 'no-console': { + description: + "We don't want a bunch of console noise, but you can use the `logger` from `@superset-ui/core` when there's a reason to.", + }, + }; + + try { + // Run OXC with JSON format + console.log('Running OXC linter...'); + const oxlintOutput = execSync('npx oxlint --format json', { + encoding: 'utf8', + maxBuffer: 50 * 1024 * 1024, // 50MB buffer for large outputs + stdio: ['pipe', 'pipe', 'ignore'], // Ignore stderr to avoid error output + }); + + const results = JSON.parse(oxlintOutput); + + // Process OXC JSON output + const metricsByRule = {}; + let occurrencesData = []; + + // OXC JSON format has diagnostics array + if (results.diagnostics && Array.isArray(results.diagnostics)) { + results.diagnostics.forEach(diagnostic => { + // Extract rule ID from code like "eslint(no-unused-vars)" or "eslint-plugin-unicorn(no-new-array)" + const codeMatch = diagnostic.code?.match( + /^(?:eslint(?:-plugin-(\w+))?\()([^)]+)\)$/, + ); + let ruleId = diagnostic.code || 'unknown'; + + if (codeMatch) { + const plugin = codeMatch[1]; + const rule = codeMatch[2]; + ruleId = plugin ? `${plugin}/${rule}` : rule; + } Review Comment: ### Monolithic function needs decomposition <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? The runOxlintAndProcess function is too large and handles multiple responsibilities including running linters, processing results, and uploading to Google Sheets. ###### Why this matters Large functions are harder to maintain, test, and understand. This violates the Single Responsibility Principle and makes the code more prone to bugs. ###### Suggested change ∙ *Feature Preview* Split the function into smaller, focused functions: ```javascript async function runOxlintAndProcess() { const oxlintResults = await runOxlint(); const eslintResults = await runEslint(); const processedData = processLintResults(oxlintResults, eslintResults); await uploadToGoogleSheets(processedData); } ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e8d322e8-7776-43c8-960c-c2659579f2a3/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e8d322e8-7776-43c8-960c-c2659579f2a3?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e8d322e8-7776-43c8-960c-c2659579f2a3?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e8d322e8-7776-43c8-960c-c2659579f2a3?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/e8d322e8-7776-43c8-960c-c2659579f2a3) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:3d2f33ef-9cc1-4f5e-9e34-73949db9207c --> [](3d2f33ef-9cc1-4f5e-9e34-73949db9207c) ########## superset-frontend/packages/superset-ui-demo/storybook/stories/plugins/plugin-chart-table/testData.ts: ########## @@ -29,7 +26,7 @@ import { // eslint-disable-next-line import/extensions import birthNamesJson from './birthNames.json'; -export const birthNames = birthNamesJson as TableChartProps; +export const birthNames = birthNamesJson as unknown as TableChartProps; Review Comment: ### Unsafe double type assertion bypasses type safety <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? Using double type assertion (as unknown as) bypasses TypeScript's type checking and forces a potentially unsafe cast from JSON data to TableChartProps. ###### Why this matters This creates runtime risk if the JSON structure doesn't match TableChartProps interface, potentially causing property access errors or unexpected behavior. It also eliminates compile-time validation that could catch data structure mismatches early. ###### Suggested change ∙ *Feature Preview* Replace with proper type validation or parsing. Either validate the JSON structure at runtime with a type guard function, or use a library like zod for schema validation: ```typescript function isTableChartProps(obj: any): obj is TableChartProps { // Add validation logic for required properties return obj && typeof obj === 'object' /* add more checks */; } export const birthNames = isTableChartProps(birthNamesJson) ? birthNamesJson : /* fallback or throw error */; ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8847f7be-cd98-4d06-a6b4-c95c8ea3b039/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8847f7be-cd98-4d06-a6b4-c95c8ea3b039?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8847f7be-cd98-4d06-a6b4-c95c8ea3b039?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8847f7be-cd98-4d06-a6b4-c95c8ea3b039?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/8847f7be-cd98-4d06-a6b4-c95c8ea3b039) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:81588f2e-ad34-4a0b-983d-60272d3270c8 --> [](81588f2e-ad34-4a0b-983d-60272d3270c8) ########## superset-frontend/src/explore/components/controls/ComparisonRangeLabel.tsx: ########## @@ -144,7 +144,8 @@ export const ComparisonRangeLabel = ({ ); const startDateDayjs = extendedDayjs(parseDttmToDate(startDate)); if ( - startDateDayjs.isSameOrBefore(parsedDateDayjs) || + startDateDayjs.isBefore(parsedDateDayjs) || + startDateDayjs.isSame(parsedDateDayjs) || Review Comment: ### Redundant date comparison methods <sub></sub> <details> <summary>Tell me more</summary> ###### What is the issue? Two separate method calls are used to check if a date is before or equal to another date, when a single method call could achieve the same result. ###### Why this matters This creates unnecessary method invocations and reduces code readability. The dayjs library provides `isSameOrBefore()` method that combines both checks in a single operation. ###### Suggested change ∙ *Feature Preview* Replace the two separate calls with the single `isSameOrBefore()` method: ```typescript startDateDayjs.isSameOrBefore(parsedDateDayjs) ``` ###### Provide feedback to improve future suggestions [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/33cbab4b-fe8d-4614-af12-02c83d3284aa/upvote) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/33cbab4b-fe8d-4614-af12-02c83d3284aa?what_not_true=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/33cbab4b-fe8d-4614-af12-02c83d3284aa?what_out_of_scope=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/33cbab4b-fe8d-4614-af12-02c83d3284aa?what_not_in_standard=true) [](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/33cbab4b-fe8d-4614-af12-02c83d3284aa) </details> <sub> 💬 Looking for more details? Reply to this comment to chat with Korbit. </sub> <!--- korbi internal id:137d156e-9bb0-48b9-a7c8-59cf213b6764 --> [](137d156e-9bb0-48b9-a7c8-59cf213b6764) -- 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]
