fix(desktop): separate env files by context to prevent renderer crashes by saddlepaddle · Pull Request #421 · superset-sh/superset

saddlepaddle · 2025-12-18T23:28:04Z

Summary

Fixed blank screen issue in production builds caused by ...process.env spread being bundled into renderer code
Split env.ts into context-specific files:
- src/main/env.main.ts - Main process env vars (can use t3-env with ...process.env)
- src/renderer/env.renderer.ts - Renderer env vars (uses Zod with Vite-injected values)
- src/shared/env.shared.ts - Shared env vars (only NODE_ENV, works in both contexts)
Updated CI workflow to pass PostHog env vars at build time

Why this works

Vite can only replace individual process.env.X property accesses at build time. The spread ...process.env requires an actual object at runtime, which doesn't exist in the browser. By splitting the env files:

Main process uses t3-env with runtime process.env (Node.js has this)
Renderer uses Zod validation on build-time injected values (Vite define block)
Shared code imports from env.shared.ts which only accesses process.env.NODE_ENV (replaced by Vite)

Test plan

Verified typecheck passes
Tested deep links work in production build
Verified PostHog initializes with key from CI secrets
Confirmed app doesn't crash on launch

🤖 Generated with Claude Code

Summary by CodeRabbit

Chores
- Configured production environment settings and analytics tracking integration for the desktop application.
- Updated environment variable configuration to support customizable API and web service URLs.
- Reorganized internal environment configuration management for improved application stability.

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

coderabbitai · 2025-12-18T23:28:16Z

Warning

Rate limit exceeded

@saddlepaddle has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 14 minutes and 18 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between 1d6043f and a1eba49.

📒 Files selected for processing (12)

.github/workflows/release-desktop.yml (2 hunks)
apps/desktop/src/lib/electron-app/factories/app/setup.ts (1 hunks)
apps/desktop/src/main/env.main.ts (2 hunks)
apps/desktop/src/main/lib/api-client.ts (1 hunks)
apps/desktop/src/main/lib/auth/auth-service.ts (1 hunks)
apps/desktop/src/main/lib/auth/deep-link-handler.ts (1 hunks)
apps/desktop/src/main/lib/auto-updater.ts (1 hunks)
apps/desktop/src/main/lib/sound-paths.ts (1 hunks)
apps/desktop/src/main/lib/terminal-history.ts (1 hunks)
apps/desktop/src/renderer/env.renderer.ts (1 hunks)
apps/desktop/src/renderer/lib/posthog.ts (1 hunks)
apps/desktop/src/shared/constants.ts (1 hunks)

Walkthrough

The desktop app's environment variable handling is reorganized into separate modules for main process, renderer, and shared contexts. PostHog analytics configuration is added to the release workflow, and environment variables are exposed through the Electron build configuration.

Changes

Cohort / File(s)	Summary
Release Workflow & Build Configuration `.github/workflows/release-desktop.yml`, `apps/desktop/electron.vite.config.ts`	Adds `environment: production` to the release job and injects PostHog configuration (NEXT_PUBLIC_POSTHOG_KEY, NEXT_PUBLIC_POSTHOG_HOST) as environment variables. Exposes NEXT_PUBLIC_API_URL and NEXT_PUBLIC_WEB_URL through Electron Vite config with defaults ("https://api.superset.sh", "https://app.superset.sh").
New Environment Modules `apps/desktop/src/shared/env.shared.ts`, `apps/desktop/src/main/env.main.ts`, `apps/desktop/src/renderer/env.renderer.ts`	Creates three new Zod-based environment modules: shared (NODE_ENV only), main (Node.js context), and renderer (browser context with PostHog and API/web URLs). Each module validates environment variables with appropriate defaults and schema constraints.
Import Path Updates (Main Process) `apps/desktop/src/main/lib/api-client.ts`, `apps/desktop/src/main/lib/auth/auth-service.ts`, `apps/desktop/src/main/lib/auth/deep-link-handler.ts`, `apps/desktop/src/main/lib/auto-updater.ts`, `apps/desktop/src/main/lib/sound-paths.ts`, `apps/desktop/src/main/lib/terminal-history.ts`	Updates import paths from various relative paths to `../env.main` or `../../env.main` to use the new main process environment module.
Import Path Updates (Shared/Renderer) `apps/desktop/src/lib/electron-router-dom.ts`, `apps/desktop/src/lib/electron-app/factories/app/setup.ts`, `apps/desktop/src/shared/constants.ts`	Updates imports to reference `shared/env.shared` and `main/env.main` respectively. Reformats PROTOCOL_SCHEME constant to multi-line style in constants.ts.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20–25 minutes

Import path consistency: Verify all 9 files correctly reference the new env module locations and that old import paths are fully replaced.
Environment schema validation: Review the Zod schemas in env.shared.ts, env.main.ts, and env.renderer.ts for correctness of defaults, enum values, and optional field handling (especially NEXT_PUBLIC_POSTHOG_KEY).
Build-time inlining: Confirm that renderer env values are correctly marked as build-time constants and that Vite's define mechanism in electron.vite.config.ts properly inlines them.
Release workflow integration: Verify PostHog env variable injection in the release workflow does not interfere with existing CI/CD steps and that the production environment context is properly established.

Possibly related PRs

chore(desktop): bump version to 0.0.19 #370: Also updates desktop build configuration and injects NODE_ENV into electron builds, aligning with environment variable management.
release setup #131: Modifies the same release workflow file (.github/workflows/release-desktop.yml), introducing foundational release setup extended by this PR.
Update .env.example #377: Addresses NEXT_PUBLIC_API_URL and related public environment variables, directly overlapping with this PR's exposure of those URLs.

Poem

🐰 Our modules hop in three's now clear,
Shared, main, and renderer drawing near,
PostHog hops to production ground,
Environment paths reorganized 'round!
Zod schemas validate each little thing—
A structured warren for the desktop to spring! 🌿

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title accurately describes the main change: splitting environment files by context to fix renderer crashes in production builds.
Description check	✅ Passed	The description covers the fix, implementation approach, technical rationale, and test results, but omits the required template sections like 'Related Issues' and 'Type of Change'.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

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.}

- Remove env.ts imports from shared code (constants.ts, electron-router-dom.ts) that gets bundled into renderer - env.ts uses ...process.env which doesn't work in browser context - Use process.env.NODE_ENV directly which Vite replaces at build time - Add NEXT_PUBLIC_API_URL and NEXT_PUBLIC_WEB_URL to Vite's define block - Re-enable PostHog now that the root cause is fixed The issue was that renderer code imported from shared/constants.ts which imported env.ts. When bundled for the browser, the ...process.env spread in env.ts would fail since process.env doesn't exist in browser context. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

- Add environment: production to build job to access env secrets - Pass NEXT_PUBLIC_POSTHOG_KEY and NEXT_PUBLIC_POSTHOG_HOST to compile step 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Create separate env files for each Electron process: - src/main/env.ts: Main process env with t3-env (runtime process.env) - src/renderer/env.ts: Renderer env with Zod (build-time Vite defines) This makes it clear which env file to use based on where the code runs: - Main process code imports from main/env - Renderer code imports from renderer/env - Shared code should NOT import env (use process.env.NODE_ENV directly) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Renamed files: - src/main/env.ts → src/main/env.main.ts - src/renderer/env.ts → src/renderer/env.renderer.ts Added: - src/shared/env.shared.ts - For shared code that only needs NODE_ENV The naming convention makes it explicit which context each env file is for: - env.main.ts: Main process (t3-env + runtime process.env) - env.renderer.ts: Renderer process (Zod + build-time Vite defines) - env.shared.ts: Shared code (only vars defined in both contexts) 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

Update constants.ts and electron-router-dom.ts to import from env.shared.ts instead of using process.env.NODE_ENV directly. This ensures all env var access goes through one of the three typed env files (env.main.ts, env.renderer.ts, env.shared.ts). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>

github-actions · 2025-12-18T23:36:31Z

🧹 Preview Cleanup Complete

The following preview resources have been cleaned up:

Service	Status
Database (Neon)	✅

Thank you for your contribution! 🎉

Preview resources have been processed for cleanup

coderabbitai

Actionable comments posted: 1

🧹 Nitpick comments (2)

apps/desktop/src/renderer/env.renderer.ts (2)
10-10: Minor documentation inconsistency in comment.

The comment references src/main/env.ts but the actual file is src/main/env.main.ts.
🔎 Suggested fix
- * For main process env vars, use src/main/env.ts instead.
+ * For main process env vars, use src/main/env.main.ts instead.
14-22: Consider using z.url() for PostHog host validation.

The NEXT_PUBLIC_POSTHOG_HOST field uses z.string().default() while the API URLs use z.url().default(). Since the PostHog host is also a URL, consider using z.url() for consistency and stricter validation.
🔎 Proposed refactor
 	NEXT_PUBLIC_POSTHOG_KEY: z.string().optional(),
-	NEXT_PUBLIC_POSTHOG_HOST: z.string().default("https://us.i.posthog.com"),
+	NEXT_PUBLIC_POSTHOG_HOST: z.url().default("https://us.i.posthog.com"),
 });

📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between a5e5859 and 1d6043f.

📒 Files selected for processing (14)

.github/workflows/release-desktop.yml (2 hunks)
apps/desktop/electron.vite.config.ts (1 hunks)
apps/desktop/src/lib/electron-app/factories/app/setup.ts (1 hunks)
apps/desktop/src/lib/electron-router-dom.ts (1 hunks)
apps/desktop/src/main/env.main.ts (2 hunks)
apps/desktop/src/main/lib/api-client.ts (1 hunks)
apps/desktop/src/main/lib/auth/auth-service.ts (1 hunks)
apps/desktop/src/main/lib/auth/deep-link-handler.ts (1 hunks)
apps/desktop/src/main/lib/auto-updater.ts (1 hunks)
apps/desktop/src/main/lib/sound-paths.ts (1 hunks)
apps/desktop/src/main/lib/terminal-history.ts (1 hunks)
apps/desktop/src/renderer/env.renderer.ts (1 hunks)
apps/desktop/src/shared/constants.ts (2 hunks)
apps/desktop/src/shared/env.shared.ts (1 hunks)

🧰 Additional context used

📓 Path-based instructions (6)

apps/desktop/**/*.{ts,tsx,js,jsx}

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

apps/desktop/**/*.{ts,tsx,js,jsx}: 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.

Files:

apps/desktop/src/main/lib/api-client.ts
apps/desktop/src/shared/env.shared.ts
apps/desktop/src/shared/constants.ts
apps/desktop/electron.vite.config.ts
apps/desktop/src/main/lib/auth/auth-service.ts
apps/desktop/src/lib/electron-router-dom.ts
apps/desktop/src/main/lib/auth/deep-link-handler.ts
apps/desktop/src/main/lib/terminal-history.ts
apps/desktop/src/renderer/env.renderer.ts
apps/desktop/src/main/lib/sound-paths.ts
apps/desktop/src/main/lib/auto-updater.ts
apps/desktop/src/lib/electron-app/factories/app/setup.ts
apps/desktop/src/main/env.main.ts

**/*.{ts,tsx}