apache
fix(DatasourceEditor): add mount guards and fix async race conditions
User description
SUMMARY
Fixes async race conditions and setState-on-unmounted-component warnings in DatasourceEditor, and eliminates test flakiness through proper async patterns and test utilities.
Production Fixes:
- Implements per-request AbortController pattern for 4 async methods (formatQuery, formatSql, syncMetadata, fetchUsageData)
- Adds mount guards to prevent setState on unmounted components during abort cleanup
- Fixes race condition where
componentWillUnmount()triggers abort, but catch blocks execute AFTER unmount completes - Prevents memory leaks from pending async operations
Test Improvements:
- Eliminates CI timeouts by replacing fireEvent with proper userEvent patterns
- Adds comprehensive test utilities (createProps factory, asyncRender, cleanup helpers)
- Removes describe() blocks following avoid nesting when testing best practices
- Adds abort/unmount tests with console.error spies to verify no React warnings
- Adds mock setup utilities to prevent test hangs from missing API mocks
Key Technical Details:
-
Hybrid AbortController + Mount Guards Pattern:
- AbortController cancels network requests and prevents success path execution
- Mount guards (
isComponentMounted,isMountedRef) prevent setState in abort cleanup paths that run during/after unmount - Guards placed ONLY on AbortError cleanup and finally blocks, NOT on normal success/error paths
-
Lifecycle Management:
isComponentMountedinitialized tofalsein constructor- Set to
trueincomponentDidMount() - Set to
falseFIRST incomponentWillUnmount()before aborting requests - DatasetUsageTab uses
isMountedRefwith useEffect cleanup
-
Test Stability:
- Replaced all fireEvent.click() with userEvent.click() for realistic user interactions
- Use fastRender + findBy* queries instead of custom timeouts
- All tests use default 20s timeout with proper async patterns
- Added cleanupAsyncOperations() utility to flush microtasks and animation frames
BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
TESTING INSTRUCTIONS
- Verify no React warnings:
npm run test -- DatasourceEditor.test.tsx
- All 19 tests should pass
- No "setState on unmounted component" warnings in console
- Manual test for abort behavior: - Open dataset editor - Navigate to Usage tab (triggers fetchUsageData) - Immediately close modal - Check browser console - no React warnings
- Manual test for formatQuery abort: - Open dataset in SQL mode - Type SQL and click "Format SQL" button - Quickly click again before first request completes - Verify no duplicate formatting or warnings
- CI stability: - Run tests multiple times to verify no flakiness: npm run test -- DatasourceEditor --maxWorkers=1 --runInBand
ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in SIP-59)
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
CodeAnt-AI Description
Prevent stale async updates and request leaks in the dataset editor and usage tab
What Changed
- Cancels in‑flight SQL formatting, metadata sync, and usage fetch requests when a new request starts or the editor closes, so only the latest result is applied and closed editors no longer surface errors
- Guards dataset usage pagination and loading state so they are not updated after the usage tab unmounts or a request is aborted, avoiding stale spinners and suppressing “Error fetching charts” toasts on cancelled loads
- Strengthens dataset editor tests to cover async cancellation, metric and column editing, and currency configuration without timer or DOM leakage
Impact
✅ Stable dataset editor when switching tabs or closing while data is loading
✅ SQL formatting and metadata sync reflect only the latest edits
✅ No spurious usage errors when requests are cancelled or tabs change
💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
@codeant-ai ask: Your question here
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
@codeant-ai ask: Can you suggest a safer alternative to storing this secret?
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
@codeant-ai: Your feedback here
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
@codeant-ai: Do not flag unused imports.
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
@codeant-ai: review
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.
Code Review Agent Run #1c432f
Actionable Suggestions - 0
Review Details
-
Files reviewed - 5 · Commit Range:
5fb5e20..5fb5e20- superset-frontend/src/components/Datasource/components/DatasourceEditor/DatasourceEditor.jsx
- superset-frontend/src/components/Datasource/components/DatasourceEditor/components/DatasetUsageTab/index.tsx
- superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditor.test.tsx
- superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditorCurrency.test.tsx
- superset-frontend/src/components/Datasource/components/DatasourceEditor/tests/DatasourceEditorRTL.test.tsx
-
Files skipped - 0
-
Tools
- 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 Default Agent You can customize the agent settings here or contact your Bito workspace admin at [email protected].
Documentation & Help
AI Code Review powered by 
![bito-code-review[bot] avatar](https://avatars.githubusercontent.com/in/1061978?v=4 "Published by a pub.dev verified publisher"){kind=small}
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
Interaction Diagram by Bito
sequenceDiagram
participant USER as User Interface
participant MODAL as DatasourceModal
participant EDITOR as DatasourceEditor<br/>🔄 Updated | ●●● High
participant USAGE as DatasetUsageTab<br/>🔄 Updated | ●●○ Medium
participant API as SupersetClient API
participant TOAST as Toast System
Note over EDITOR, USAGE: Mount tracking prevents<br/>state updates on unmounted components
USER->>MODAL: Open dataset editor
MODAL->>EDITOR: Initialize component
EDITOR->>EDITOR: Set isComponentMounted = true
EDITOR->>USAGE: Render usage tab
USAGE->>USAGE: Set isMountedRef = true
USAGE->>API: Fetch chart usage data
API-->>USAGE: Return chart data
USAGE->>TOAST: Show success/error messages
USER->>MODAL: Close editor
EDITOR->>EDITOR: Set isComponentMounted = false
USAGE->>USAGE: Set isMountedRef = false
Critical path: User Interface->DatasourceModal->DatasourceEditor->DatasetUsageTab->SupersetClient API->Toast System
Note: The changes add mount tracking to DatasourceEditor and DatasetUsageTab components to prevent state updates after unmounting. This prevents React warnings and potential memory leaks when async operations complete after component cleanup.
🎪 Showtime deployed environment on GHA for 5fb5e20
• Environment: http://34.208.211.230:8080 (admin/admin) • Lifetime: 48h auto-cleanup • Updates: New commits create fresh environments automatically
@sadpandajoe is there somewhere we're encountering this error that we should be looking at for validation (of the fix, and the problem)?
@sadpandajoe is there somewhere we're encountering this error that we should be looking at for validation (of the fix, and the problem)?
@rusackas yes, when looking at the jest tests, we should see a reduction of the following errors compared to master. This right now only cleans up some files, i'll have follow up prs for other ones, I just want to have smaller PRs to review. Here are the errors we're cleaning up:
/app/superset-frontend/node_modules/react-dom/cjs/react-dom.development.js:3901 throw Error( "The
documentglobal was defined when React was initialized, but is not defined anymore. This can happen in a test environment if a component schedules an update from an asynchronous callback, but the test has already finished running. To solve this, you can either unmount the component at the end of your test (and ensure that any asynchronous operations get canceled incomponentWillUnmount), or you can change the test itself to be asynchronous." ); ^ [Error: Thedocumentglobal was defined when React was initialized, but is not defined anymore. This can happen in a test environment if a component schedules an update from an asynchronous callback, but the test has already finished running. To solve this, you can either unmount the component at the end of your test (and ensure that any asynchronous operations get canceled incomponentWillUnmount), or you can change the test itself to be asynchronous.]
Problem is you can see them but the jest tests are still green so these errors are either being silenced or something else is going on. You can see that the jest tests for shard 1 still has them, but it doesn't look like it's related to these files I've touched.
The PR description mentions using requestAnimationFrame for scroll, but I don't see that in the diff. The code only shows mount guards added to handleFetchCharts. Question: Was the scroll improvement removed from this PR? The description should match the actual changes.
![codeant-ai-for-open-source[bot] avatar](https://avatars.githubusercontent.com/u/151810221?v=4 "Published by a pub.dev verified publisher"){kind=small}
Nitpicks 🔍
| 🔒 No security issues identified |
⚡ Recommended areas for review
|
🎪 Showtime deployed environment on GHA for 1b767b5
• Environment: http://35.163.200.64:8080 (admin/admin) • Lifetime: 48h auto-cleanup • Updates: New commits create fresh environments automatically
🎪 Showtime deployed environment on GHA for a6f1e0a
• Environment: http://35.89.113.150:8080 (admin/admin) • Lifetime: 48h auto-cleanup • Updates: New commits create fresh environments automatically
🎪 Showtime deployed environment on GHA for b0c1634
• Environment: http://44.244.13.202:8080 (admin/admin) • Lifetime: 48h auto-cleanup • Updates: New commits create fresh environments automatically
⚠️ DEPRECATED WORKFLOW ⚠️
@sadpandajoe This workflow is deprecated! Please use the new Superset Showtime system instead:
- Replace "testenv-up" label with "🎪 trigger-start"
- Better reliability and easier management
- See https://github.com/mistercrunch/superset-showtime for details
Processing your ephemeral environment request here. Action: up. More information on how to use or configure ephemeral environments
@sadpandajoe Ephemeral environment spinning up at http://54.213.138.236:8080. Credentials are 'admin'/'admin'. Please allow several minutes for bootstrapping and startup.