korbit-ai[bot] commented on code in PR #34123:
URL: https://github.com/apache/superset/pull/34123#discussion_r2197855246
##########
superset-frontend/plugins/legacy-preset-chart-deckgl/src/DeckGLContainer.tsx:
##########
@@ -154,13 +134,11 @@ export const DeckGLContainer = memo(
glContextRef.current = context.gl;
}}
>
- {props.mapStyle?.startsWith(MAPBOX_LAYER_PREFIX) && (
- <StaticMap
- preserveDrawingBuffer
- mapStyle={props.mapStyle || 'light'}
- mapboxApiAccessToken={props.mapboxApiAccessToken}
- />
- )}
+ <StaticMap
+ preserveDrawingBuffer
+ mapStyle={props.mapStyle || 'light'}
+ mapboxApiAccessToken={props.mapboxApiAccessToken}
+ />
Review Comment:
### Unconditional StaticMap Rendering Without Token Check <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The StaticMap component is now unconditionally rendered, which will fail if
mapboxApiAccessToken is not provided, as it's required for Mapbox-based maps.
###### Why this matters
This will cause runtime errors when attempting to render maps without a
valid Mapbox API token, leading to map visualization failures.
###### Suggested change ∙ *Feature Preview*
Restore the conditional rendering check for Mapbox maps to prevent errors
when API token is missing:
```typescript
{props.mapboxApiAccessToken && (
<StaticMap
preserveDrawingBuffer
mapStyle={props.mapStyle || 'light'}
mapboxApiAccessToken={props.mapboxApiAccessToken}
/>
)}
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/60c314b4-7e26-4b44-b295-916f3f9bba65/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/60c314b4-7e26-4b44-b295-916f3f9bba65?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/60c314b4-7e26-4b44-b295-916f3f9bba65?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/60c314b4-7e26-4b44-b295-916f3f9bba65?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/60c314b4-7e26-4b44-b295-916f3f9bba65)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:4565323c-f7fe-4ff7-8880-652b5f3b2775 -->
[](4565323c-f7fe-4ff7-8880-652b5f3b2775)
##########
superset-frontend/packages/superset-ui-core/src/validator/validateMapboxStylesUrl.ts:
##########
@@ -19,28 +19,18 @@
import { t } from '../translation';
-const VALIDE_OSM_URLS = ['https://tile.osm', 'https://tile.openstreetmap'];
-
/**
* Validate a [Mapbox styles
URL](https://docs.mapbox.com/help/glossary/style-url/)
- * or a title server URL.
* @param v
*/
export default function validateMapboxStylesUrl(v: unknown) {
- if (typeof v === 'string') {
- const trimmed_v = v.trim();
- if (
- typeof v === 'string' &&
- trimmed_v.length > 0 &&
- (trimmed_v.startsWith('mapbox://styles/') ||
- trimmed_v.startsWith('tile://http') ||
- VALIDE_OSM_URLS.some(s => trimmed_v.startsWith(s)))
- ) {
- return false;
- }
+ if (
+ typeof v === 'string' &&
+ v.trim().length > 0 &&
+ v.trim().startsWith('mapbox://styles/')
Review Comment:
### Redundant String Trimming <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Redundant trim() calls causing unnecessary string operations. The string is
trimmed twice for each validation.
###### Why this matters
Multiple trim() operations on the same string value create unnecessary CPU
overhead, especially when processing large numbers of URLs in a loop or
high-frequency validation scenarios.
###### Suggested change ∙ *Feature Preview*
Store the trimmed value in a variable and reuse it:
```typescript
const trimmed = v.trim();
if (
typeof v === 'string' &&
trimmed.length > 0 &&
trimmed.startsWith('mapbox://styles/')
)
```
###### Provide feedback to improve future suggestions
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/fb7cc033-7112-4edf-9b1b-c8f14c413113/upvote)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/fb7cc033-7112-4edf-9b1b-c8f14c413113?what_not_true=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/fb7cc033-7112-4edf-9b1b-c8f14c413113?what_out_of_scope=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/fb7cc033-7112-4edf-9b1b-c8f14c413113?what_not_in_standard=true)
[](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/fb7cc033-7112-4edf-9b1b-c8f14c413113)
</details>
<sub>
💬 Looking for more details? Reply to this comment to chat with Korbit.
</sub>
<!--- korbi internal id:235154d7-1024-44a5-ba52-43397a3b51ec -->
[](235154d7-1024-44a5-ba52-43397a3b51ec)
--
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]
