Skip to content

fix(frontend): handle null/undefined path in ensureAppRoot#39940

Merged
hainenber merged 3 commits into
apache:masterfrom
Abdulrehman-PIAIC80387:fix/ensure-app-root-undefined-39855
May 23, 2026
Merged

fix(frontend): handle null/undefined path in ensureAppRoot#39940
hainenber merged 3 commits into
apache:masterfrom
Abdulrehman-PIAIC80387:fix/ensure-app-root-undefined-39855

Conversation

@Abdulrehman-PIAIC80387
Copy link
Copy Markdown
Contributor

SUMMARY

ensureAppRoot() in superset-frontend/src/utils/pathUtils.ts calls path.startsWith(...) on its input without first checking that path is defined. When a caller (e.g. Menu.tsx rendering the brand link) passes theme.brandLogoHref that is undefined, the entire app crashes during initialization with:

TypeError: Cannot read properties of undefined (reading 'startsWith')

This happens in real-world setups where a user partially overrides THEME_DEFAULT / THEME_DARK in superset_config.py — Python replaces the entire token dict rather than merging, so tokens like brandLogoHref end up unset.

Fix: widen the parameter type to accept null | undefined and fall back to the application root (or '/') when the path is missing. The app stays alive instead of going to a blank screen.

Fixes #39855

BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF

Before:

After:

TESTING INSTRUCTIONS

Add this to superset_config.py:

THEME_DARK = {
    "token": {"brandLogoUrl": "/static/assets/images/superset-logo-horiz.png"},
    "algorithm": "dark",
}
  1. Restart Superset and switch the UI to the Dark theme.
  2. Before the fix: the page is blank and the console shows Cannot read properties of undefined (reading 'startsWith').
  3. After the fix: the page loads normally; the brand link defaults to the application root.

ADDITIONAL INFORMATION

@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented May 7, 2026

Code Review Agent Run #4c3ab1

Actionable Suggestions - 0
Additional Suggestions - 1
  • superset-frontend/src/utils/pathUtils.ts - 1
    • Incorrect null check logic · Line 45-45
      The condition `if (!path)` treats empty strings as falsy, but the comment specifies handling only null or undefined. This alters behavior for empty string inputs (e.g., from `ensureAppRoot('')` returning `${applicationRoot()}/` to returning `applicationRoot()`), which may affect usages like `ensureAppRoot(item.url || '')` in RightMenu.tsx.
      Code suggestion
       @@ -44,3 +44,3 @@
      - export function ensureAppRoot(path: string | null | undefined): string {
      -   if (!path) {
      -     return applicationRoot() || '/';
      + export function ensureAppRoot(path: string | null | undefined): string {
      +   if (path == null) {
      +     return applicationRoot() || '/';
Review Details
  • Files reviewed - 1 · Commit Range: 1ab0fba..1ab0fba
    • superset-frontend/src/utils/pathUtils.ts
  • Files skipped - 0
  • Tools
    • Eslint (Linter) - ✔︎ Successful
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@dosubot dosubot Bot added the change:frontend Requires changing the frontend label May 7, 2026
@sadpandajoe sadpandajoe force-pushed the fix/ensure-app-root-undefined-39855 branch from 1ab0fba to 25c468b Compare May 13, 2026 17:49
@codecov
Copy link
Copy Markdown

codecov Bot commented May 13, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 64.16%. Comparing base (ac5e8f1) to head (0a98f45).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #39940      +/-   ##
==========================================
- Coverage   64.16%   64.16%   -0.01%     
==========================================
  Files        2591     2591              
  Lines      138283   138285       +2     
  Branches    32083    32085       +2     
==========================================
+ Hits        88726    88727       +1     
- Misses      48027    48028       +1     
  Partials     1530     1530              
Flag Coverage Δ
javascript 67.03% <100.00%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented May 15, 2026

Code Review Agent Run #83899d

Actionable Suggestions - 0
Additional Suggestions - 1
  • superset-frontend/src/utils/pathUtils.ts - 1
    • Code-Comment Mismatch · Line 45-45
      The code checks `!path` which includes empty strings, but the comment specifies only null or undefined should fall back to the application root. To align with the documentation, use `path == null` instead.
Review Details
  • Files reviewed - 1 · Commit Range: 25c468b..25c468b
    • superset-frontend/src/utils/pathUtils.ts
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • Eslint (Linter) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

Comment thread superset-frontend/src/utils/pathUtils.ts Outdated
@bito-code-review
Copy link
Copy Markdown
Contributor

The PR diff shows a change to ensureAppRoot in pathUtils.ts to handle null/undefined paths by returning the application root. The PR comments file contains only a header row with no actual comments. I cannot analyze review discussions or suggestions since there are no data rows in the comments file.

superset-frontend/src/utils/pathUtils.ts

export function ensureAppRoot(path: string | null | undefined): string {
  if (!path) {
    return applicationRoot() || '/';
  }
  if (SAFE_ABSOLUTE_URL_RE.test(path) || path.startsWith('//')) {
    return path;
  }

@Abdulrehman-PIAIC80387 Abdulrehman-PIAIC80387 force-pushed the fix/ensure-app-root-undefined-39855 branch from 25c468b to 5242926 Compare May 19, 2026 07:21
@netlify
Copy link
Copy Markdown

netlify Bot commented May 19, 2026

Deploy Preview for superset-docs-preview ready!

Name Link
🔨 Latest commit ee4172e
🔍 Latest deploy log https://app.netlify.com/projects/superset-docs-preview/deploys/6a113c09a39c3900090441d4
😎 Deploy Preview https://deploy-preview-39940--superset-docs-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.
🤖 Make changes Run an agent on this branch

To edit notification comments on pull requests, go to your Netlify project configuration.

@Abdulrehman-PIAIC80387 Abdulrehman-PIAIC80387 force-pushed the fix/ensure-app-root-undefined-39855 branch from 5242926 to 0a98f45 Compare May 19, 2026 07:22
@Abdulrehman-PIAIC80387
Copy link
Copy Markdown
Contributor Author

@hainenber Thanks for the review — addressed:

  • Reordered so the security/passthrough check runs before any return (as you suggested).
  • Switched !pathpath == null per the Bito secondary suggestion, so empty strings still get the app-root prefix (matches the docstring "null or undefined").
  • Added two tests covering null / undefined input (the missing coverage codecov flagged).

pathUtils.test.ts: 19/19 pass locally.

@bito-code-review
Copy link
Copy Markdown
Contributor

bito-code-review Bot commented May 21, 2026

Code Review Agent Run #b8d5ee

Actionable Suggestions - 0
Review Details
  • Files reviewed - 2 · Commit Range: 81d65cf..0a98f45
    • superset-frontend/src/utils/pathUtils.test.ts
    • superset-frontend/src/utils/pathUtils.ts
  • Files skipped - 0
  • Tools
    • Whispers (Secret Scanner) - ✔︎ Successful
    • Detect-secrets (Secret Scanner) - ✔︎ Successful
    • Eslint (Linter) - ✔︎ Successful

Bito Usage Guide

Commands

Type the following command in the pull request comment and save the comment.

  • /review - Manually triggers a full AI review.

  • /pause - Pauses automatic reviews on this pull request.

  • /resume - Resumes automatic reviews.

  • /resolve - Marks all Bito-posted review comments as resolved.

  • /abort - Cancels all in-progress reviews.

Refer to the documentation for additional commands.

Configuration

This repository uses Superset You can customize the agent settings here or contact your Bito workspace admin at evan@preset.io.

Documentation & Help

AI Code Review powered by Bito Logo

@hainenber
Copy link
Copy Markdown
Contributor

LGTM. Thanks for your contribution! @Abdulrehman-PIAIC80387

@hainenber hainenber merged commit 816794b into apache:master May 23, 2026
61 checks passed
@bito-code-review
Copy link
Copy Markdown
Contributor

Bito Automatic Review Skipped – PR Already Merged

Bito scheduled an automatic review for this pull request, but the review was skipped because this PR was merged before the review could be run.
No action is needed if you didn't intend to review it. To get a review, you can type /review in a comment and save it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:frontend Requires changing the frontend size/S

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[6.1.0rc3] Bug in the "Dark" theme (?)

2 participants