refactor: replace runtime isElectron() with build-time isDesktop constant#8710
Merged
christian-byrne merged 1 commit intomainfrom Feb 8, 2026
Merged
Conversation
christian-byrne
requested review from
a team,
benceruleanlu and
pythongosssss
as code owners
dosubot
bot
added
the
size:L
🎭 Playwright Tests:
|
🎨 Storybook Build Status✅ Build completed successfully! ⏰ Completed at: 02/07/2026, 11:25:49 PM UTC 🔗 Links🎉 Your Storybook is ready for review! |
Contributor
📝 WalkthroughWalkthroughThe PR replaces runtime Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
Bundle Size ReportSummary
Category Glance Per-category breakdownApp Entry Points — 22.2 kB (baseline 22.2 kB) • 🟢 -22 BMain entry bundles and manifests
Status: 1 added / 1 removed Graph Workspace — 854 kB (baseline 854 kB) • 🟢 -102 BGraph editor runtime, canvas, workflow orchestration
Status: 1 added / 1 removed Views & Navigation — 68.8 kB (baseline 69 kB) • 🟢 -210 BTop-level views, pages, and routed surfaces
Status: 11 added / 11 removed Panels & Settings — 409 kB (baseline 409 kB) • 🟢 -244 BConfiguration panels, inspectors, and settings screens
Status: 12 added / 12 removed User & Accounts — 16 kB (baseline 16.1 kB) • 🟢 -90 BAuthentication, profile, and account management bundles
Status: 6 added / 6 removed Editors & Dialogs — 751 B (baseline 781 B) • 🟢 -30 BModals, dialogs, drawers, and in-app editors
Status: 1 added / 1 removed UI Components — 36.6 kB (baseline 36.6 kB) • 🟢 -80 BReusable component library chunks
Status: 9 added / 9 removed Data & Services — 2.11 MB (baseline 2.11 MB) • 🟢 -149 BStores, services, APIs, and repositories
Status: 13 added / 13 removed Utilities & Hooks — 237 kB (baseline 237 kB) • 🟢 -114 BHelpers, composables, and utility bundles
Status: 18 added / 18 removed Vendor & Third-Party — 8.77 MB (baseline 8.77 MB) • ⚪ 0 BExternal libraries and shared vendor chunks
Status: 8 added / 8 removed Other — 7.15 MB (baseline 7.15 MB) • 🟢 -1.03 kBBundles that do not match a named category
Status: 82 added / 82 removed |
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/views/templates/BaseViewTemplate.vue (1)
25-49:⚠️ Potential issue | 🟠 MajorGuard against missing
electronAPI()to avoid runtime crashes.
isDesktopis compile-time, butelectronAPI()can still be null (e.g., preload failure or dev runs). Add a null check before callingchangeTheme.Suggested fix
onMounted(async () => { if (isDesktop) { await nextTick() - electronAPI().changeTheme({ + const api = electronAPI() + if (!api) return + api.changeTheme({ ...(dark ? darkTheme : lightTheme), height: topMenuRef.value?.getBoundingClientRect().height ?? 0 }) } })src/stores/electronDownloadStore.ts (1)
53-64:⚠️ Potential issue | 🟡 MinorGuard actions when
DownloadManageris unavailable.
With the non-null assertions, any unexpected call path in web builds will throw. Consider a defensive guard (or a controlled error) to avoid hard crashes.🛡️ Suggested defensive guard
- }) => DownloadManager!.startDownload(url, savePath, filename) - const pause = (url: string) => DownloadManager!.pauseDownload(url) - const resume = (url: string) => DownloadManager!.resumeDownload(url) - const cancel = (url: string) => DownloadManager!.cancelDownload(url) + }) => { + if (!DownloadManager) return + return DownloadManager.startDownload(url, savePath, filename) + } + const pause = (url: string) => { + if (!DownloadManager) return + return DownloadManager.pauseDownload(url) + } + const resume = (url: string) => { + if (!DownloadManager) return + return DownloadManager.resumeDownload(url) + } + const cancel = (url: string) => { + if (!DownloadManager) return + return DownloadManager.cancelDownload(url) + }As per coding guidelines: Implement proper error handling; Provide user-friendly and actionable error messages.
🧹 Nitpick comments (1)
src/platform/updates/common/releaseStore.test.ts (1)
21-28: ResetmockData.isDesktopin the rootbeforeEachto avoid state leakage.
Because the hoisted object is mutable, later suites can inherit the previous state unless it’s reset explicitly.♻️ Suggested reset for test isolation
beforeEach(() => { setActivePinia(createTestingPinia({ stubActions: false })) vi.resetAllMocks() mockSystemStatsState.reset() + mockData.isDesktop = true })Based on learnings: Applies to **/*.test.ts : Keep module mocks contained; do not use global mutable state within test files; use
vi.hoisted()if necessary for per-test Arrange phase manipulation.
…tant Replace all runtime isElectron() function calls with the build-time isDesktop constant from @/platform/distribution/types. This aligns desktop detection with the existing distribution system (__DISTRIBUTION__ compile-time constant set by Vite), enabling dead-code elimination in non-desktop builds. - Remove isElectron() from envUtil.ts - Update isNativeWindow() to use isDesktop - Migrate 30 files from isElectron() to isDesktop - Guard electronAPI() calls behind isDesktop checks - Update 7 test files to use vi.hoisted mock pattern - Add DISTRIBUTION=desktop to dev:electron script Amp-Thread-ID: https://ampcode.com/threads/T-019c3604-e5ba-7062-9561-a97d8f427725
christian-byrne
force-pushed
the
refactor/replace-is-electron-with-is-desktop
branch
from
0f57c00 to
fa32f19
Compare
|
Warning Review the following alerts detected in dependencies. According to your organization's Security Policy, it is recommended to resolve "Warn" alerts. Learn more about Socket for GitHub.
|
Contributor
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/stores/electronDownloadStore.ts (1)
53-64:⚠️ Potential issue | 🟠 MajorNon-null assertions on
DownloadManager!can crash in non-desktop builds.The
start,pause,resume, andcancelmethods use non-null assertions (DownloadManager!), but these methods are exported and could be called by consumers in web builds whereDownloadManagerisundefined. This contradicts the PR objective to "guard electronAPI() calls behind isDesktop checks... to avoid crashes in non-desktop builds."🛡️ Proposed fix: Add guards or throw descriptive errors
Option 1: Early return with no-op (silent)
const start = ({ url, savePath, filename }: { url: string savePath: string filename: string - }) => DownloadManager!.startDownload(url, savePath, filename) - const pause = (url: string) => DownloadManager!.pauseDownload(url) - const resume = (url: string) => DownloadManager!.resumeDownload(url) - const cancel = (url: string) => DownloadManager!.cancelDownload(url) + }) => DownloadManager?.startDownload(url, savePath, filename) + const pause = (url: string) => DownloadManager?.pauseDownload(url) + const resume = (url: string) => DownloadManager?.resumeDownload(url) + const cancel = (url: string) => DownloadManager?.cancelDownload(url)Option 2: Throw descriptive error (explicit)
+ function assertDesktop(): asserts DownloadManager is NonNullable<typeof DownloadManager> { + if (!DownloadManager) { + throw new Error('Download operations are only available in desktop builds') + } + } + const start = ({ url, savePath, filename }: { url: string savePath: string filename: string - }) => DownloadManager!.startDownload(url, savePath, filename) + }) => { + assertDesktop() + return DownloadManager.startDownload(url, savePath, filename) + }
benceruleanlu
approved these changes
Feb 8, 2026
christian-byrne
deleted the
refactor/replace-is-electron-with-is-desktop
branch
This was referenced Feb 8, 2026
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
2 participants
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.
Summary
Replace all runtime
isElectron()function calls with the build-timeisDesktopconstant from@/platform/distribution/types, enabling dead-code elimination in non-desktop builds.Changes
isElectron()detection (checkingwindow.electronAPI) to the compile-timeisDesktopconstant (driven by__DISTRIBUTION__Vite define). RemoveisElectronfromenvUtil.ts. UpdateisNativeWindow()to useisDesktop. GuardelectronAPI()calls behindisDesktopchecks in stores. Update 7 test files to usevi.hoisted+ getter mock pattern for per-testisDesktoptoggling. AddDISTRIBUTION=desktoptodev:electronscript.Review Focus
electronDownloadStore.tsnow guards the top-levelelectronAPI()call behindisDesktopto prevent crashes on non-desktop builds.vi.hoistedwith a getter to allow per-test toggling of theisDesktopvalue.as ElectronAPIcast inenvUtil.ts,:class="[]"inBaseViewTemplate.vue,@ts-expect-errorinModelLibrarySidebarTab.vue.isElectroncallsites to use isDesktop distribution type #6122 and PR refactor: replace runtimeisElectronwith buildtimeisDesktop#7374 obsolete.┆Issue is synchronized with this Notion page by Unito