fix(import): fix file picker not opening on repeated import attempts COMPASS-10565#7981
Draft
johnjackweir wants to merge 2 commits intomainfrom
Draft
fix(import): fix file picker not opening on repeated import attempts COMPASS-10565#7981johnjackweir wants to merge 2 commits intomainfrom
johnjackweir wants to merge 2 commits intomainfrom
Conversation
…OMPASS-10565 Two issues were preventing the import file picker from working reliably: 1. In createElectronFileInputBackend, when showOpenDialog rejected (e.g. on macOS ARM), the catch handler silently swallowed the error without notifying listeners. This left the import modal in a stuck invisible state. Fix: notify listeners with an empty file list on error so the modal can clean up via the onCancel handler. 2. When the import was re-triggered while the modal was stuck, openImport reset the state to the same shape (isOpen: true, fileName: '', errors: []). React reused the existing ImportFileInput without remounting, so the mount-only autoOpen effect never re-fired. Fix: add an openId counter to the import state that increments on each OPEN action and use it as a React key to force remounting ImportFileInput. Also fixed openImportFileDialog in crud-store to accept and forward the origin parameter for correct telemetry tracking. Co-authored-by: Jack Weir <johnjackweir@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes reliability issues with the “Add Data → Import JSON or CSV file” flow by ensuring file dialog failures and repeated open attempts always result in a clean cancel/close path, and by correcting telemetry origin attribution.
Changes:
- Add an
openIdcounter to import Redux state and use it as a Reactkeyto force remounting the auto-opening file input on each OPEN. - Ensure Electron file dialog backend notifies listeners with
[]whenshowOpenDialogrejects (instead of silently swallowing the error). - Update CRUD store
openImportFileDialogto accept/forward anoriginparameter rather than hardcoding'empty-state'.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/compass-import-export/src/modules/import.ts | Adds openId to state and increments it on each OPEN to support deterministic remounting behavior. |
| packages/compass-import-export/src/modules/import.spec.ts | Adds reducer-level tests validating openId increments across opens (with and without closing). |
| packages/compass-import-export/src/components/import-modal.tsx | Uses openId as a key for ImportFileInput when auto-opening, forcing the native dialog to re-trigger on re-open. |
| packages/compass-crud/src/stores/crud-store.ts | Forwards the correct origin in the emitted open-import event. |
| packages/compass-components/src/components/file-picker-dialog.tsx | Calls listeners with [] when the Electron dialog promise rejects, avoiding stuck UI state. |
| packages/compass-components/src/components/file-picker-dialog.spec.tsx | Adds a test ensuring listeners are called with [] on dialog rejection. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
Fixes two interrelated bugs that prevented the "Add Data >> Import JSON or CSV file" feature from working reliably, particularly on macOS ARM:
Bug 1 – Silent error swallowing in file dialog backend: When
electron.dialog.showOpenDialogrejected (e.g. due to@electron/remoteissues on macOS ARM), the.catch(() => { /* ignore */ })handler silently swallowed the error without notifying listeners. This left the import modal in a stuck invisible state (isOpen: true, rendering insidedisplay: none), where the user sees "nothing happened."Fix: The catch handler now notifies listeners with an empty file list (
[]) on error, which triggers theonCancelcallback and properly closes the modal.Bug 2 – File dialog not re-opening on second attempt: When the import was re-triggered while the modal was stuck (or after a failed first attempt where state wasn't cleaned up),
openImportdispatchedOPENwhich reset the state to the same shape (isOpen: true, fileName: '', errors: []). React reused the existingImportFileInputcomponent without remounting it, so the mount-onlyautoOpeneffect (useEffect([], [])) never re-fired, and the native file dialog never opened.Fix: Added an
openIdcounter to the import Redux state that increments on eachOPENaction. ThisopenIdis used as a Reactkeyon theImportFileInputcomponent, forcing React to remount it on every new import attempt, which re-triggers theautoOpeneffect.Bug 3 – Incorrect telemetry origin:
openImportFileDialogincrud-store.tsalways hardcodedorigin: 'empty-state'regardless of the actual origin ('crud-toolbar'or'empty-state').Fix: The method now accepts and forwards the
originparameter.Checklist
Motivation and Context
Users on macOS ARM reported that the import file picker would either do nothing on first attempt, or completely stop responding on subsequent attempts within the same session (COMPASS-10565).
Open Questions
FilePickerDialogcomponent is marked as@deprecatedin favor ofFileSelector. A follow-up migration could further improve robustness.Types of changes