korbit-ai[bot] commented on code in PR #34160:
URL: https://github.com/apache/superset/pull/34160#discussion_r2205491141
##########
superset-frontend/src/utils/datasourceUtils.ts:
##########
@@ -16,7 +16,21 @@
* specific language governing permissions and limitations
* under the License.
*/
-export const getDatasourceAsSaveableDataset = source => ({
+import { Dataset } from '@superset-ui/chart-controls';
+
+interface SaveableDataset {
+ columns: any[];
+ name: string;
+ dbId?: number;
+ sql: string;
+ catalog?: string | null;
+ schema?: string;
+ templateParams?: string;
+}
+
+export const getDatasourceAsSaveableDataset = (
+ source: Partial<Dataset>,
+): SaveableDataset => ({
Review Comment:
### Input type too permissive for output guarantees <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The function returns a SaveableDataset with required properties but accepts
a Partial<Dataset>, which could lead to runtime errors if required properties
are missing.
###### Why this matters
There's a potential mismatch between the input type (which may lack required
properties) and the output type (which requires certain properties). This could
cause runtime errors that TypeScript can't catch.
###### Suggested change ∙ *Feature Preview*
Either strengthen the input type constraints or handle missing properties
explicitly:
```typescript
export const getDatasourceAsSaveableDataset = (
source: Pick<Dataset, 'columns' | 'sql' | 'name' | 'database'>,
): SaveableDataset => ({
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/585c5c99-8e56-44fc-97e2-a115b392cd77/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/585c5c99-8e56-44fc-97e2-a115b392cd77?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/585c5c99-8e56-44fc-97e2-a115b392cd77?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/585c5c99-8e56-44fc-97e2-a115b392cd77?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/585c5c99-8e56-44fc-97e2-a115b392cd77)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:7bfdefe5-7802-432b-847c-f49ff07a7f96 -->
[](7bfdefe5-7802-432b-847c-f49ff07a7f96)
##########
superset-frontend/src/utils/datasourceUtils.ts:
##########
@@ -16,7 +16,21 @@
* specific language governing permissions and limitations
* under the License.
*/
-export const getDatasourceAsSaveableDataset = source => ({
+import { Dataset } from '@superset-ui/chart-controls';
+
+interface SaveableDataset {
+ columns: any[];
+ name: string;
+ dbId?: number;
+ sql: string;
+ catalog?: string | null;
+ schema?: string;
+ templateParams?: string;
+}
+
+export const getDatasourceAsSaveableDataset = (
+ source: Partial<Dataset>,
+): SaveableDataset => ({
columns: source.columns,
Review Comment:
### Missing Columns Property Fallback <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
No null check or fallback value for the columns property, which is required
according to the interface.
###### Why this matters
If source.columns is undefined, this will cause a runtime error as the
property is required by the SaveableDataset interface.
###### Suggested change ∙ *Feature Preview*
Add a fallback for the columns property:
```typescript
columns: source.columns || [],
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b15aa931-841a-4489-ae1a-4c173bb1cb37/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b15aa931-841a-4489-ae1a-4c173bb1cb37?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b15aa931-841a-4489-ae1a-4c173bb1cb37?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b15aa931-841a-4489-ae1a-4c173bb1cb37?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/b15aa931-841a-4489-ae1a-4c173bb1cb37)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:a930c56b-39c3-4477-8872-e633b7cfeb22 -->
[](a930c56b-39c3-4477-8872-e633b7cfeb22)
##########
superset-frontend/src/utils/datasourceUtils.ts:
##########
@@ -16,7 +16,21 @@
* specific language governing permissions and limitations
* under the License.
*/
-export const getDatasourceAsSaveableDataset = source => ({
+import { Dataset } from '@superset-ui/chart-controls';
+
+interface SaveableDataset {
+ columns: any[];
+ name: string;
+ dbId?: number;
+ sql: string;
+ catalog?: string | null;
+ schema?: string;
+ templateParams?: string;
+}
Review Comment:
### Required SQL Property Inconsistency <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The 'sql' property is marked as required in SaveableDataset interface, but
the implementation provides an empty string fallback, indicating it should be
optional.
###### Why this matters
This mismatch could force consumers to provide SQL when it's not actually
required, or cause TypeScript compilation errors when valid data is provided
without SQL.
###### Suggested change ∙ *Feature Preview*
Make the 'sql' property optional in the interface to match the
implementation:
```typescript
interface SaveableDataset {
columns: any[];
name: string;
dbId?: number;
sql?: string;
catalog?: string | null;
schema?: string;
templateParams?: string;
}
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ea41d0f1-6144-4503-928a-1d09c77a1fb4/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ea41d0f1-6144-4503-928a-1d09c77a1fb4?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ea41d0f1-6144-4503-928a-1d09c77a1fb4?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ea41d0f1-6144-4503-928a-1d09c77a1fb4?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/ea41d0f1-6144-4503-928a-1d09c77a1fb4)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:ac071011-d01a-451e-8004-bb973ae600c9 -->
[](ac071011-d01a-451e-8004-bb973ae600c9)
##########
superset-frontend/src/utils/getControlsForVizType.ts:
##########
@@ -0,0 +1,58 @@
+/**
+ * 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.
+ */
+
+import memoizeOne from 'memoize-one';
+import {
+ isControlPanelSectionConfig,
+ ControlStateMapping,
+ ControlPanelConfig,
+} from '@superset-ui/chart-controls';
+import { getChartControlPanelRegistry } from '@superset-ui/core';
+import { controls } from '../explore/controls';
+
+const memoizedControls = memoizeOne(
+ (vizType: string, controlPanel: ControlPanelConfig): ControlStateMapping => {
+ const controlsMap: ControlStateMapping = {};
+ (controlPanel?.controlPanelSections || [])
+ .filter(isControlPanelSectionConfig)
+ .forEach(section => {
+ section.controlSetRows.forEach(row => {
+ row.forEach(control => {
+ if (!control) return;
+ if (typeof control === 'string') {
+ // For now, we have to look in controls.jsx to get the config
for some controls.
+ // Once everything is migrated out, delete this if statement.
+ controlsMap[control] = controls[control];
+ } else if (control.name && control.config) {
+ // condition needed because there are elements, e.g. <hr /> in
some control configs (I'm looking at you, FilterBox!)
+ controlsMap[control.name] = control.config;
+ }
+ });
+ });
+ });
+ return controlsMap;
+ },
+);
+
+const getControlsForVizType = (vizType: string): ControlStateMapping => {
+ const controlPanel = getChartControlPanelRegistry().get(vizType);
+ return memoizedControls(vizType, controlPanel);
Review Comment:
### Missing Error Handling for Invalid Viz Types <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The code doesn't handle the case where
getChartControlPanelRegistry().get(vizType) returns undefined for an invalid or
unregistered vizType.
###### Why this matters
This could lead to runtime errors when attempting to process undefined
controlPanel values, potentially causing the application to crash.
###### Suggested change ∙ *Feature Preview*
Add error handling for undefined control panels:
```typescript
const getControlsForVizType = (vizType: string): ControlStateMapping => {
const controlPanel = getChartControlPanelRegistry().get(vizType);
if (!controlPanel) {
return {}; // or throw new Error(`Invalid visualization type:
${vizType}`);
}
return memoizedControls(vizType, controlPanel);
};
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/93e5a515-2c77-49aa-9bad-ace25d5091b1/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/93e5a515-2c77-49aa-9bad-ace25d5091b1?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/93e5a515-2c77-49aa-9bad-ace25d5091b1?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/93e5a515-2c77-49aa-9bad-ace25d5091b1?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/93e5a515-2c77-49aa-9bad-ace25d5091b1)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:2d2c12a7-e8b3-4bc1-bb91-12cae561306e -->
[](2d2c12a7-e8b3-4bc1-bb91-12cae561306e)
##########
superset-frontend/src/utils/DebouncedMessageQueue.ts:
##########
@@ -18,33 +18,49 @@
*/
import { debounce } from 'lodash';
+interface DebouncedMessageQueueConfig {
+ callback?: (events: any[]) => void;
+ sizeThreshold?: number;
+ delayThreshold?: number;
+}
+
class DebouncedMessageQueue {
+ private queue: any[];
+
+ private sizeThreshold: number;
+
+ private delayThreshold: number;
+
+ private callback: (events: any[]) => void;
+
+ private trigger: () => void;
+
constructor({
callback = () => {},
sizeThreshold = 1000,
delayThreshold = 1000,
- }) {
+ }: DebouncedMessageQueueConfig = {}) {
this.queue = [];
this.sizeThreshold = sizeThreshold;
this.delayThreshold = delayThreshold;
- this.trigger = debounce(this.trigger.bind(this), this.delayThreshold);
+ this.trigger = debounce(this.triggerQueue.bind(this), this.delayThreshold);
this.callback = callback;
}
- append(eventData) {
+ append(eventData: any): void {
this.queue.push(eventData);
this.trigger();
}
Review Comment:
### Unvalidated Input in Queue <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The append method accepts any type of data without validation, which could
lead to arbitrary data being stored and processed in the queue.
###### Why this matters
Without input validation, malicious data could be injected into the queue
and potentially executed or processed by the callback function, leading to
potential security vulnerabilities if the data contains malicious code or
oversized payloads.
###### Suggested change ∙ *Feature Preview*
```typescript
append(eventData: any): void {
if (!eventData || typeof eventData === 'function') {
throw new Error('Invalid event data');
}
this.queue.push(eventData);
this.trigger();
}
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6b0d1c2b-293e-41c6-a09a-74aa9a0a0839/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6b0d1c2b-293e-41c6-a09a-74aa9a0a0839?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6b0d1c2b-293e-41c6-a09a-74aa9a0a0839?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6b0d1c2b-293e-41c6-a09a-74aa9a0a0839?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/6b0d1c2b-293e-41c6-a09a-74aa9a0a0839)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:8643b78a-6d84-42c5-8429-0c5bbca95a1e -->
[](8643b78a-6d84-42c5-8429-0c5bbca95a1e)
##########
superset-frontend/src/utils/DebouncedMessageQueue.ts:
##########
@@ -18,33 +18,49 @@
*/
import { debounce } from 'lodash';
+interface DebouncedMessageQueueConfig {
+ callback?: (events: any[]) => void;
+ sizeThreshold?: number;
+ delayThreshold?: number;
+}
+
class DebouncedMessageQueue {
+ private queue: any[];
+
+ private sizeThreshold: number;
+
+ private delayThreshold: number;
+
+ private callback: (events: any[]) => void;
+
+ private trigger: () => void;
+
constructor({
callback = () => {},
sizeThreshold = 1000,
delayThreshold = 1000,
- }) {
+ }: DebouncedMessageQueueConfig = {}) {
this.queue = [];
this.sizeThreshold = sizeThreshold;
this.delayThreshold = delayThreshold;
- this.trigger = debounce(this.trigger.bind(this), this.delayThreshold);
+ this.trigger = debounce(this.triggerQueue.bind(this), this.delayThreshold);
this.callback = callback;
}
- append(eventData) {
+ append(eventData: any): void {
this.queue.push(eventData);
this.trigger();
}
- trigger() {
+ private triggerQueue(): void {
if (this.queue.length > 0) {
const events = this.queue.splice(0, this.sizeThreshold);
this.callback.call(null, events);
Review Comment:
### Unprotected callback execution <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The callback invocation lacks error handling, despite being an existing
point of error handling concern since it executes user-provided code.
###### Why this matters
If the callback throws an error, it will break the queue processing and
potentially leave remaining items unprocessed, affecting the reliability of the
message queue.
###### Suggested change ∙ *Feature Preview*
```typescript
private triggerQueue(): void {
if (this.queue.length > 0) {
const events = this.queue.splice(0, this.sizeThreshold);
try {
this.callback.call(null, events);
} catch (error) {
// Re-add failed events back to the front of the queue
this.queue.unshift(...events);
console.error('Error processing message queue:', error);
}
// If there are remaining items, call it again.
if (this.queue.length > 0) {
this.triggerQueue();
}
}
}
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/fedcff2f-8cfe-41a9-b338-cb548ad1600b/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/fedcff2f-8cfe-41a9-b338-cb548ad1600b?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/fedcff2f-8cfe-41a9-b338-cb548ad1600b?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/fedcff2f-8cfe-41a9-b338-cb548ad1600b?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/fedcff2f-8cfe-41a9-b338-cb548ad1600b)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:27b5e027-2896-4ce9-baea-b1ec33425846 -->
[](27b5e027-2896-4ce9-baea-b1ec33425846)
##########
superset-frontend/src/utils/DebouncedMessageQueue.ts:
##########
@@ -18,33 +18,49 @@
*/
import { debounce } from 'lodash';
+interface DebouncedMessageQueueConfig {
+ callback?: (events: any[]) => void;
+ sizeThreshold?: number;
+ delayThreshold?: number;
+}
+
class DebouncedMessageQueue {
+ private queue: any[];
+
+ private sizeThreshold: number;
+
+ private delayThreshold: number;
+
+ private callback: (events: any[]) => void;
+
+ private trigger: () => void;
+
constructor({
callback = () => {},
sizeThreshold = 1000,
delayThreshold = 1000,
- }) {
+ }: DebouncedMessageQueueConfig = {}) {
Review Comment:
### Unexplained Magic Numbers in Defaults <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Magic numbers 1000 for thresholds lack context about their units or
significance.
###### Why this matters
Without understanding if these numbers represent milliseconds, bytes, or
items, developers must dig through implementation details or documentation.
###### Suggested change ∙ *Feature Preview*
```typescript
private static readonly DEFAULT_SIZE_THRESHOLD = 1000; // maximum items per
batch
private static readonly DEFAULT_DELAY_THRESHOLD = 1000; // milliseconds
constructor({
callback = () => {},
sizeThreshold = DebouncedMessageQueue.DEFAULT_SIZE_THRESHOLD,
delayThreshold = DebouncedMessageQueue.DEFAULT_DELAY_THRESHOLD,
}: DebouncedMessageQueueConfig = {}) {
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/45684f75-ab0a-4db2-898f-3dd9e687b1e6/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/45684f75-ab0a-4db2-898f-3dd9e687b1e6?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/45684f75-ab0a-4db2-898f-3dd9e687b1e6?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/45684f75-ab0a-4db2-898f-3dd9e687b1e6?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/45684f75-ab0a-4db2-898f-3dd9e687b1e6)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:49231325-8fc1-494e-9078-4d0ca853f598 -->
[](49231325-8fc1-494e-9078-4d0ca853f598)
##########
superset-frontend/src/utils/getControlsForVizType.ts:
##########
@@ -0,0 +1,58 @@
+/**
+ * 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.
+ */
+
+import memoizeOne from 'memoize-one';
+import {
+ isControlPanelSectionConfig,
+ ControlStateMapping,
+ ControlPanelConfig,
+} from '@superset-ui/chart-controls';
+import { getChartControlPanelRegistry } from '@superset-ui/core';
+import { controls } from '../explore/controls';
+
+const memoizedControls = memoizeOne(
+ (vizType: string, controlPanel: ControlPanelConfig): ControlStateMapping => {
+ const controlsMap: ControlStateMapping = {};
+ (controlPanel?.controlPanelSections || [])
+ .filter(isControlPanelSectionConfig)
+ .forEach(section => {
+ section.controlSetRows.forEach(row => {
+ row.forEach(control => {
Review Comment:
### Nested forEach Loops Reduce Readability <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Nested forEach loops with arrow functions make the code harder to follow.
The nested structure creates multiple levels of indentation and callback
functions.
###### Why this matters
Deep nesting reduces code scanability and makes it harder to track the flow
of data transformation. This affects maintainability and debugging.
###### Suggested change ∙ *Feature Preview*
```typescript
for (const row of section.controlSetRows) {
for (const control of row) {
// ... rest of the code
}
}
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/983473f2-efbc-4e94-b0f4-ad67591880ee/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/983473f2-efbc-4e94-b0f4-ad67591880ee?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/983473f2-efbc-4e94-b0f4-ad67591880ee?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/983473f2-efbc-4e94-b0f4-ad67591880ee?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/983473f2-efbc-4e94-b0f4-ad67591880ee)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:5545d241-0b43-407e-bb50-00fa99636842 -->
[](5545d241-0b43-407e-bb50-00fa99636842)
##########
superset-frontend/src/utils/DebouncedMessageQueue.ts:
##########
@@ -18,33 +18,49 @@
*/
import { debounce } from 'lodash';
+interface DebouncedMessageQueueConfig {
+ callback?: (events: any[]) => void;
+ sizeThreshold?: number;
+ delayThreshold?: number;
+}
+
class DebouncedMessageQueue {
+ private queue: any[];
+
+ private sizeThreshold: number;
+
+ private delayThreshold: number;
+
+ private callback: (events: any[]) => void;
+
+ private trigger: () => void;
+
constructor({
callback = () => {},
sizeThreshold = 1000,
delayThreshold = 1000,
- }) {
+ }: DebouncedMessageQueueConfig = {}) {
this.queue = [];
Review Comment:
### Inefficient array resizing <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Array resizing during splice operations can be inefficient for large queues
with frequent operations.
###### Why this matters
Memory reallocation and copying during array operations can impact
performance when handling large amounts of data.
###### Suggested change ∙ *Feature Preview*
Pre-allocate array capacity when possible or consider using a more efficient
data structure like a circular buffer for queue operations:
```typescript
private queue: any[] = new Array(this.sizeThreshold);
private head = 0;
private tail = 0;
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/feca26ca-1bc1-4779-a146-21ee3e3be3bd/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/feca26ca-1bc1-4779-a146-21ee3e3be3bd?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/feca26ca-1bc1-4779-a146-21ee3e3be3bd?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/feca26ca-1bc1-4779-a146-21ee3e3be3bd?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/feca26ca-1bc1-4779-a146-21ee3e3be3bd)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:4740db1f-5c06-48fc-bbd5-bcc238f83942 -->
[](4740db1f-5c06-48fc-bbd5-bcc238f83942)
##########
superset-frontend/src/utils/getControlsForVizType.ts:
##########
@@ -0,0 +1,58 @@
+/**
+ * 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.
+ */
+
+import memoizeOne from 'memoize-one';
+import {
+ isControlPanelSectionConfig,
+ ControlStateMapping,
+ ControlPanelConfig,
+} from '@superset-ui/chart-controls';
+import { getChartControlPanelRegistry } from '@superset-ui/core';
+import { controls } from '../explore/controls';
+
+const memoizedControls = memoizeOne(
+ (vizType: string, controlPanel: ControlPanelConfig): ControlStateMapping => {
+ const controlsMap: ControlStateMapping = {};
+ (controlPanel?.controlPanelSections || [])
+ .filter(isControlPanelSectionConfig)
+ .forEach(section => {
+ section.controlSetRows.forEach(row => {
+ row.forEach(control => {
+ if (!control) return;
+ if (typeof control === 'string') {
+ // For now, we have to look in controls.jsx to get the config
for some controls.
+ // Once everything is migrated out, delete this if statement.
Review Comment:
### Temporary Solution Creating Technical Debt <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The code contains a technical debt marker indicating a temporary solution
that depends on an external controls.jsx file.
###### Why this matters
Temporary solutions can become permanent, leading to maintenance issues and
unclear dependencies. This violates the Open-Closed Principle as future changes
require modifying existing code.
###### Suggested change ∙ *Feature Preview*
Create an interface for control configurations and implement a proper
dependency injection or factory pattern:
```typescript
interface ControlConfigProvider {
getConfig(controlName: string): ControlConfig;
}
const getControlsForVizType = (
vizType: string,
configProvider: ControlConfigProvider
): ControlStateMapping => {
// Use configProvider instead of direct controls import
}
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a53c81c4-9fc3-4e3d-84c8-7cb30220d9f9/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a53c81c4-9fc3-4e3d-84c8-7cb30220d9f9?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a53c81c4-9fc3-4e3d-84c8-7cb30220d9f9?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a53c81c4-9fc3-4e3d-84c8-7cb30220d9f9?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/a53c81c4-9fc3-4e3d-84c8-7cb30220d9f9)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:871b931f-9585-406c-880e-c3d9f04b7823 -->
[](871b931f-9585-406c-880e-c3d9f04b7823)
--
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]
