Re: [PR] fix(bootstrapData): Missing application_root data throws an error [superset]

via GitHub Wed, 13 Aug 2025 11:49:45 -0700


korbit-ai[bot] commented on code in PR #34680:
URL: https://github.com/apache/superset/pull/34680#discussion_r2274317241



##########
superset-frontend/src/utils/getBootstrapData.ts:
##########
@@ -33,11 +33,15 @@ export default function getBootstrapData(): BootstrapData {
   return cachedBootstrapData ?? DEFAULT_BOOTSTRAP_DATA;
 }
 
-const APPLICATION_ROOT_NO_TRAILING_SLASH =
-  getBootstrapData().common.application_root.replace(/\/$/, '');
+const APPLICATION_ROOT_NO_TRAILING_SLASH = (
+  getBootstrapData().common.application_root ??
+  DEFAULT_BOOTSTRAP_DATA.common.application_root
+).replace(/\/$/, '');

Review Comment:
   ### Premature Bootstrap Data Access <sub>![category 
Functionality](https://img.shields.io/badge/Functionality-0284c7)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   The constant is initialized immediately upon module import, but 
getBootstrapData() might not have access to the DOM element '#app' at that 
point.
   
   
   ###### Why this matters
   This could lead to always using the default values if the script runs before 
the DOM is ready, even when bootstrap data is actually present in the DOM.
   
   ###### Suggested change ∙ *Feature Preview*
   Move the constant initialization into the respective functions to ensure DOM 
is ready when accessing bootstrap data:
   ```typescript
   export function applicationRoot(): string {
     return (getBootstrapData().common.application_root ?? 
       DEFAULT_BOOTSTRAP_DATA.common.application_root).replace(/\/$/, '');
   }
   
   export function staticAssetsPrefix(): string {
     return (getBootstrapData().common.static_assets_prefix ?? 
       DEFAULT_BOOTSTRAP_DATA.common.static_assets_prefix).replace(/\/$/, '');
   }
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2fe3e5b0-b0b7-4726-b18c-cb330f2180e9/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2fe3e5b0-b0b7-4726-b18c-cb330f2180e9?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2fe3e5b0-b0b7-4726-b18c-cb330f2180e9?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2fe3e5b0-b0b7-4726-b18c-cb330f2180e9?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/2fe3e5b0-b0b7-4726-b18c-cb330f2180e9)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:586e44d1-eda1-4951-acc6-a5f6e5d5443a -->
   
   
   [](586e44d1-eda1-4951-acc6-a5f6e5d5443a)



##########
superset-frontend/src/utils/getBootstrapData.ts:
##########
@@ -33,11 +33,15 @@
   return cachedBootstrapData ?? DEFAULT_BOOTSTRAP_DATA;
 }
 
-const APPLICATION_ROOT_NO_TRAILING_SLASH =
-  getBootstrapData().common.application_root.replace(/\/$/, '');
+const APPLICATION_ROOT_NO_TRAILING_SLASH = (
+  getBootstrapData().common.application_root ??
+  DEFAULT_BOOTSTRAP_DATA.common.application_root
+).replace(/\/$/, '');
 
-const STATIC_ASSETS_PREFIX_NO_TRAILING_SLASH =
-  getBootstrapData().common.static_assets_prefix.replace(/\/$/, '');
+const STATIC_ASSETS_PREFIX_NO_TRAILING_SLASH = (
+  getBootstrapData().common.static_assets_prefix ??
+  DEFAULT_BOOTSTRAP_DATA.common.static_assets_prefix
+).replace(/\/$/, '');

Review Comment:
   ### Duplicate Path Normalization Logic <sub>![category 
Design](https://img.shields.io/badge/Design-0d9488)</sub>
   
   <details>
     <summary>Tell me more</summary>
   
   ###### What is the issue?
   Duplicate logic pattern for handling paths with fallbacks and trailing slash 
removal.
   
   
   ###### Why this matters
   Violates DRY principle and makes maintenance more error-prone. If the path 
handling logic needs to change, it must be updated in multiple places.
   
   ###### Suggested change ∙ *Feature Preview*
   Extract the common logic into a reusable function:
   ```typescript
   const normalizePathWithFallback = (
     path: string | undefined,
     fallback: string
   ): string => (path ?? fallback).replace(/\/$/, '');
   
   const APPLICATION_ROOT_NO_TRAILING_SLASH = normalizePathWithFallback(
     getBootstrapData().common.application_root,
     DEFAULT_BOOTSTRAP_DATA.common.application_root
   );
   ```
   
   
   ###### Provide feedback to improve future suggestions
   [![Nice 
Catch](https://img.shields.io/badge/👍%20Nice%20Catch-71BC78)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/cbd74534-3914-4050-b57a-d65af799306c/upvote)
 
[![Incorrect](https://img.shields.io/badge/👎%20Incorrect-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/cbd74534-3914-4050-b57a-d65af799306c?what_not_true=true)
  [![Not in 
Scope](https://img.shields.io/badge/👎%20Out%20of%20PR%20scope-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/cbd74534-3914-4050-b57a-d65af799306c?what_out_of_scope=true)
 [![Not in coding 
standard](https://img.shields.io/badge/👎%20Not%20in%20our%20standards-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/cbd74534-3914-4050-b57a-d65af799306c?what_not_in_standard=true)
 
[![Other](https://img.shields.io/badge/👎%20Other-white)](https://app.korbit.ai/feedback/aa91ff46-6083-4491-9416-b83dd1994b51/cbd74534-3914-4050-b57a-d65af799306c)
   </details>
   
   <sub>
   
   💬 Looking for more details? Reply to this comment to chat with Korbit.
   </sub>
   
   <!--- korbi internal id:d133088a-8fe4-4b78-ac2b-4a7abb6b0aff -->
   
   
   [](d133088a-8fe4-4b78-ac2b-4a7abb6b0aff)



-- 
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]

Re: [PR] fix(bootstrapData): Missing application_root data throws an error [superset]

Reply via email to