Skip to content

fix(desktop): disable confirm-on-quit in development mode#539

Merged
Kitenite merged 2 commits into
superset-sh:mainfrom
andreasasprou:disable-warn-on-exit-in-dev
Dec 30, 2025
Merged

fix(desktop): disable confirm-on-quit in development mode#539
Kitenite merged 2 commits into
superset-sh:mainfrom
andreasasprou:disable-warn-on-exit-in-dev

Conversation

@andreasasprou
Copy link
Copy Markdown
Contributor

@andreasasprou andreasasprou commented Dec 29, 2025

Why

What / How

Skip the quit confirmation dialog when NODE_ENV === "development":

const isDev = process.env.NODE_ENV === "development";
const shouldConfirm = !skipConfirmation && !isDev && getConfirmOnQuitSetting();

The setting continues to work normally in production builds.

Files

  • apps/desktop/src/main/index.ts

Testing

  • bun run typecheck
  • Manual: verified quit confirmation is skipped during dev hot-reload

Summary by CodeRabbit

  • Improvements
    • Development mode no longer displays quit confirmation dialogs, streamlining the workflow and preventing interruptions during iterative development and automatic reloads. This keeps confirmation behavior unchanged in non-development environments.

✏️ Tip: You can customize this high-level summary in your review settings.

Skip the quit confirmation dialog when running in development mode to
avoid interrupting hot-reload during development.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 29, 2025

📝 Walkthrough

Walkthrough

The before-quit flow now detects development mode (NODE_ENV === "development") and disables the quit confirmation dialog when idle, preventing confirmation during hot-reload; existing confirmation behavior is unchanged for non-development environments.

Changes

Cohort / File(s) Change Summary
Quit Confirmation Development Guard
apps/desktop/src/main/index.ts
Added an isDev (NODE_ENV === "development") check in the before-quit idle path to skip the quit confirmation dialog during development/hot-reload; production confirmation logic remains

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Possibly related PRs

Suggested reviewers

  • Kitenite

Poem

🐰 A dev-mode hop, no quit-confirm in sight,
Hot-reload faster, no dialog to fight,
Production still cautious, keeps the goodbye cue,
In my burrow I code, and the rest hops through! 🥕

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly and specifically describes the main change: disabling the confirm-on-quit feature in development mode, which matches the core objective of the pull request.
Description check ✅ Passed The description covers the template's essential sections: clear 'Why' and 'How' explanations, files changed, and testing verification. Type of Change is missing but not critical since the conventional commit prefix 'fix' indicates it's a bug fix.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 14e16ee and 5fe9f49.

📒 Files selected for processing (1)
  • apps/desktop/src/main/index.ts
🧰 Additional context used
📓 Path-based instructions (4)
apps/desktop/**/*.{ts,tsx}

📄 CodeRabbit inference engine (apps/desktop/AGENTS.md)

apps/desktop/**/*.{ts,tsx}: For Electron interprocess communication, ALWAYS use tRPC as defined in src/lib/trpc
Use alias as defined in tsconfig.json when possible
Prefer zustand for state management if it makes sense. Do not use effect unless absolutely necessary.
For tRPC subscriptions with trpc-electron, ALWAYS use the observable pattern from @trpc/server/observable instead of async generators, as the library explicitly checks isObservable(result) and throws an error otherwise

Files:

  • apps/desktop/src/main/index.ts
**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Use type safety and avoid any types unless absolutely necessary in TypeScript files

Files:

  • apps/desktop/src/main/index.ts
apps/desktop/src/**/*.{ts,tsx}

📄 CodeRabbit inference engine (AGENTS.md)

Move Node.js functionality needed in Electron renderer to src/main/lib/ and communicate via IPC

Files:

  • apps/desktop/src/main/index.ts
apps/desktop/src/main/index.ts

📄 CodeRabbit inference engine (AGENTS.md)

Load environment variables in src/main/index.ts with override: true before any other imports in the desktop app

Files:

  • apps/desktop/src/main/index.ts
🧠 Learnings (1)
📚 Learning: 2025-12-28T01:56:39.031Z
Learnt from: CR
Repo: superset-sh/superset PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-28T01:56:39.031Z
Learning: Applies to apps/desktop/src/main/index.ts : Load environment variables in `src/main/index.ts` with `override: true` before any other imports in the desktop app

Applied to files:

  • apps/desktop/src/main/index.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Build
🔇 Additional comments (1)
apps/desktop/src/main/index.ts (1)

142-146: LGTM! Clean implementation.

The development mode detection correctly prevents the quit confirmation dialog during hot-reload. The logic is sound: shouldConfirm will be false when NODE_ENV === "development", allowing seamless development workflow while preserving the confirmation behavior in production.


Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Kitenite
Copy link
Copy Markdown
Collaborator

I found disabling it in dev settings once works. But yeah this is better for newcomers.

@Kitenite Kitenite merged commit 42dc9ec into superset-sh:main Dec 30, 2025
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants