refactor(electron): nestjsfy #12052
refactor(electron): nestjsfy #12052
Conversation
|
Important Review skippedReview was skipped as selected files did not have any reviewable changes. 💤 Files selected but had no reviewable changes (1)
You can disable this status message by setting the WalkthroughThis change introduces a comprehensive Electron IPC system using NestJS for process modularization and type-safe inter-process communication. It adds new modules, services, decorators, and infrastructure for main, helper, and preload processes, including build-time type generation, event and handler metadata extraction, and robust logging. Legacy types and utilities are refactored, and new IPC APIs and event interfaces are auto-generated. Changes
Sequence Diagram(s)sequenceDiagram
participant Renderer
participant Preload
participant Main
participant Helper
Renderer->>Preload: Access __apis / __events via contextBridge
Preload->>Main: Invoke IPC handlers (via metadata)
Preload->>Helper: Communicate via MessagePort (AsyncCall RPC)
Main->>Helper: Bridge Electron APIs via AsyncCall RPC
Main->>Renderer: Broadcast events via IPC channel
Helper->>Main: Request Electron APIs via RPC
Helper->>Renderer: Post events via MessagePort
Assessment against linked issues
Possibly related PRs
Suggested labels
Poem
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## canary #12052 +/- ##
==========================================
+ Coverage 56.19% 56.39% +0.20%
==========================================
Files 2665 2653 -12
Lines 127618 126971 -647
Branches 20100 20044 -56
==========================================
- Hits 71715 71611 -104
+ Misses 53732 53193 -539
+ Partials 2171 2167 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
pengx17
force-pushed
the
xp/04-24-refactor_electron_nestjsfy
branch
6 times, most recently
from
a8e3823 to
b21c275
Compare
pengx17
force-pushed
the
xp/04-24-refactor_electron_nestjsfy
branch
8 times, most recently
from
80afa01 to
be79ee8
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 27
🧹 Nitpick comments (41)
packages/frontend/core/src/modules/editor-setting/services/tray-settings.ts (1)
4-4: Avoid deep relative imports; use path aliases or package exports
The new import from../../../../../apps/electron/src-old/main/shared-state-schemais brittle and ties this module to a legacy folder structure. Consider extractingshared-state-schemainto a shared package (e.g.@affine/electron/shared-state-schema) or configuring a TypeScript path alias to simplify and stabilize the import.packages/frontend/core/src/modules/media/services/meeting-settings.ts (1)
1-4: Import path update looks correct but consider migration strategyThis change updates the import path from a package-based import (
@affine/electron/main/shared-state-schema) to a deep relative path pointing to legacy code (../../../../../apps/electron/src-old/main/shared-state-schema). While this aligns with the NestJS refactoring effort, deep relative imports can be fragile.Consider:
- Adding a TODO comment indicating this is a temporary path during migration
- Planning for a final state where deep relative paths and "src-old" references are eliminated
import type { MeetingSettingsKey, MeetingSettingsSchema, + // TODO: This is a temporary import during the Electron NestJS migration - update once migration is complete } from '../../../../../apps/electron/src-old/main/shared-state-schema';packages/frontend/core/src/desktop/dialogs/setting/general-setting/meetings/index.tsx (1)
15-15: Brittle deep relative import: consider using a Tsconfig path alias
This 7-level up relative path is prone to breakage and reduces readability. Consider adding a path alias intsconfig.json(e.g."@affine/electron/shared-state-schema") so you can import:import type { MeetingSettingsSchema } from '@affine/electron/shared-state-schema';instead of the long
../../…chain.packages/frontend/apps/electron/src/entries/preload/.gitignore (1)
1-1: Exclude generated IPC metadata file.Great addition to prevent committing build artifacts. Consider broadening this pattern (for example,
*.gen.ts) if you introduce additional generated files in the future.packages/frontend/apps/electron/tsconfig.json (1)
11-11: Restrict compilation scope tosrc.The
includearray now only referencessrc. Please verify that there are no other TypeScript files (e.g., scripts or tests) outsidesrcthat require compilation, and adjust the include/glob patterns if needed.packages/frontend/apps/electron/src/entries/main/windows/custom-theme-window.service.ts (1)
3-8: Service placeholder with unnecessary constructor.This injectable service is properly set up as a placeholder for future custom theme window functionality. However, the empty constructor can be removed as it's not adding any value.
import { Injectable } from '@nestjs/common'; @Injectable() export class CustomThemeWindowService { - constructor() {} // Custom theme window functionality will be implemented here }🧰 Tools
🪛 Biome (1.9.4)
[error] 5-5: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
packages/frontend/apps/electron/src/entries/main/windows/tab-views.service.ts (1)
1-8: Good structure, but the constructor can be removedThe service class follows NestJS conventions with the
@Injectable()decorator, which is good. The placeholder comment indicates that implementation will be added later, which is fine for a work-in-progress refactoring.However, the empty constructor serves no purpose and can be removed as flagged by the static analysis tool.
import { Injectable } from '@nestjs/common'; @Injectable() export class TabViewsService { - constructor() {} // Tab views functionality will be implemented here }🧰 Tools
🪛 Biome (1.9.4)
[error] 5-5: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
packages/frontend/apps/electron/src/entries/main/windows/context-menu.service.ts (1)
1-8: Good structure, but the constructor can be removedThe service class follows NestJS conventions with the
@Injectable()decorator. The placeholder comment effectively indicates that implementation will be added in the future.However, the empty constructor serves no purpose and can be removed as flagged by the static analysis tool.
import { Injectable } from '@nestjs/common'; @Injectable() export class ContextMenuService { - constructor() {} // Context menu functionality will be implemented here }🧰 Tools
🪛 Biome (1.9.4)
[error] 5-5: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
packages/frontend/apps/electron/src/entries/main/windows/popup.service.ts (1)
1-8: Good structure, but the constructor can be removedThe service class follows NestJS conventions with the
@Injectable()decorator. The placeholder comment effectively indicates that implementation will be added in the future.However, the empty constructor serves no purpose and can be removed as flagged by the static analysis tool.
import { Injectable } from '@nestjs/common'; @Injectable() export class PopupService { - constructor() {} // Popup window functionality will be implemented here }🧰 Tools
🪛 Biome (1.9.4)
[error] 5-5: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
packages/frontend/apps/electron/src/entries/helper/shared/types.ts (1)
4-10: Interface design is clean and well-structuredThe
WorkspaceMetainterface provides a clear structure for workspace metadata with required and optional properties. The index signature allows for flexibility when working with additional metadata properties.However, consider using a more specific type than
anyfor the index signature if the additional properties follow a pattern. This would improve type safety while maintaining flexibility.packages/frontend/apps/electron/src/entries/main/windows/utils.ts (1)
4-14: Consider using memoized platform detection functionsWhile these platform detection functions are straightforward, they will be re-evaluated on each call. Since the platform doesn't change during runtime, consider memoizing these results or converting them to constants.
-export const isMacOS = () => { - return process.platform === 'darwin'; -}; +export const isMacOS = () => process.platform === 'darwin'; +// Alternative: export const IS_MACOS = process.platform === 'darwin'; -export const isWindows = () => { - return process.platform === 'win32'; -}; +export const isWindows = () => process.platform === 'win32'; +// Alternative: export const IS_WINDOWS = process.platform === 'win32'; -export const isLinux = () => { - return process.platform === 'linux'; -}; +export const isLinux = () => process.platform === 'linux'; +// Alternative: export const IS_LINUX = process.platform === 'linux';packages/frontend/apps/electron/src/entries/helper/dialog/dialog-handler.service.ts (2)
13-22: Consider adding return type to showOpenDialog methodFor better type safety, specify the return type for this method. According to Electron documentation,
showOpenDialogreturns a promise withOpenDialogReturnValue.@IpcHandle({ scope: IpcScope.DIALOG }) - async showOpenDialog( + async showOpenDialog( _event: Electron.IpcMainInvokeEvent, options: Electron.OpenDialogOptions - ) { + ): Promise<Electron.OpenDialogReturnValue> { return await this.rpcService.rpc?.showOpenDialog(options); }
24-33: Consider adding return type to showSaveDialog methodFor better type safety, specify the return type for this method. According to Electron documentation,
showSaveDialogreturns a promise withSaveDialogReturnValue.@IpcHandle({ scope: IpcScope.DIALOG }) - async showSaveDialog( + async showSaveDialog( _event: Electron.IpcMainInvokeEvent, options: Electron.SaveDialogOptions - ) { + ): Promise<Electron.SaveDialogReturnValue> { return await this.rpcService.rpc?.showSaveDialog(options); }packages/frontend/apps/electron/src/entries/preload/helper-rpc.ts (1)
51-54: Add error handling for MessagePort promise.The helper RPC setup should include error handling for the MessagePort promise.
// Helper process RPC setup export const helperRpc = AsyncCall<HelperToRenderer>(rendererToHelperServer, { channel: helperPortPromise.then(port => createMessagePortChannel(port)), log: false, }); +// Log any errors with the helper port connection +helperPortPromise.catch(error => { + console.error('[helper-rpc] Failed to establish helper connection:', error); +});packages/frontend/apps/electron/scripts/dev.ts (1)
87-107: Type generation could fail silently in certain scenarios.While the error handling for type generation is good, consider adding a mechanism to prevent subsequent builds if type generation fails after the first successful build.
build.onStart(() => { if (hasBuiltOnce) { console.log( '[IPC Types] Source code changed, generating IPC types and meta...' ); try { execSync('yarn af electron generate-types', { stdio: 'inherit', cwd: rootDir, }); console.log( '[IPC Types] IPC types and meta generated successfully.' ); } catch (error) { console.error( '[IPC Types] Error generating IPC types and meta:', error ); + // Signal to onEnd that type generation failed + build.metafile = { ...build.metafile, typeGenerationFailed: true }; } } });packages/frontend/apps/electron/src/ipc/ipc-scanner.ts (2)
15-44: Consider enhancing error handling in scanHandlers methodThe
scanHandlersmethod effectively discovers methods decorated with the IPC handle metadata key, but could benefit from additional error handling.Consider adding try/catch blocks around the reflection operations and binding to handle potential runtime errors, especially when accessing metadata:
for (const key of methodNames) { const method = instance[key]; if (typeof method !== 'function') continue; + try { const channel = Reflect.getMetadata(IPC_HANDLE_META_KEY, method); if (!channel) continue; handlers.set(channel, method.bind(instance)); + } catch (error) { + console.error( + `Error processing IPC handler in ${instance.constructor.name}.${key}:`, + error + ); + } }
46-82: Consider adding type checking in scanEventSources methodThe
scanEventSourcesmethod relies only on the presence of asubscribemethod to identify observables, which could lead to false positives.Consider adding a more robust type check or using the
instanceofoperator with a try/catch block:if ( !eventSourceCandidate || - typeof eventSourceCandidate.subscribe !== 'function' + !(eventSourceCandidate && + typeof eventSourceCandidate.subscribe === 'function' && + (eventSourceCandidate.constructor?.name === 'Subject' || + eventSourceCandidate.constructor?.name === 'Observable' || + eventSourceCandidate.constructor?.name === 'BehaviorSubject')) ) { continue; }Also, consider adding error handling around metadata access:
- const eventMeta = Reflect.getMetadata( - IPC_EVENT_META_KEY, - instance, - propertyKey - ); + let eventMeta; + try { + eventMeta = Reflect.getMetadata( + IPC_EVENT_META_KEY, + instance, + propertyKey + ); + } catch (error) { + console.error( + `Error accessing metadata for ${instance.constructor.name}.${String(propertyKey)}:`, + error + ); + continue; + }packages/frontend/apps/electron/src/ipc/ipc-event.ts (1)
42-57: Document the trailing $ character removal behaviorThe code removes trailing
$characters from property names when constructing the event name, which is a common convention for RxJS observables, but this behavior should be documented.const eventNameForMeta = options.name ?? String(propertyKey).replace(/\$$/, ''); + // Remove trailing $ from property names, which is a common convention for RxJS observables const channelName = `${options.scope}:${eventNameForMeta}`;packages/frontend/apps/electron/src/entries/preload/api-info.ts (2)
10-10: Address the TODO about duplicated code.This TODO comment indicates code duplication that should be resolved to improve maintainability.
Consider extracting the build type validation and schema generation logic into a shared utility module that can be imported where needed.
21-26: Improve command-line argument parsing.The current implementation for extracting window name and view ID from command-line arguments could be more robust.
Consider using a more explicit parsing approach that handles potential edge cases:
- windowName: - process.argv.find(arg => arg.startsWith('--window-name='))?.split('=')[1] ?? - 'unknown', - viewId: - process.argv.find(arg => arg.startsWith('--view-id='))?.split('=')[1] ?? - 'unknown', + windowName: (() => { + const arg = process.argv.find(arg => arg.startsWith('--window-name=')); + return arg ? arg.split('=')[1] || 'unknown' : 'unknown'; + })(), + viewId: (() => { + const arg = process.argv.find(arg => arg.startsWith('--view-id=')); + return arg ? arg.split('=')[1] || 'unknown' : 'unknown'; + })(),This handles cases where a parameter might be empty (
--window-name=) more explicitly.packages/frontend/apps/electron/scripts/generate-types.ts (1)
83-95: Consider adding TypeScript compiler options validation.The script initializes the TS project but doesn't verify if the compiler options are suitable for decorator metadata processing.
Consider adding validation for required TypeScript compiler options:
function generateIpcDefinitions() { // Initialize ts-morph project const project = new Project({ tsConfigFilePath: path.resolve(electronRoot, 'tsconfig.json'), skipAddingFilesFromTsConfig: true, }); + // Verify TS compiler options + const compilerOptions = project.getCompilerOptions(); + const requiredOptions = { + emitDecoratorMetadata: true, + experimentalDecorators: true, + }; + + // Check for required compiler options + Object.entries(requiredOptions).forEach(([option, value]) => { + if (compilerOptions[option] !== value) { + console.warn( + `[WARN] TypeScript compiler option '${option}' should be set to ${value} for proper decorator processing.` + ); + } + }); // Add relevant source files project.addSourceFilesAtPaths([ path.resolve(electronRoot, 'src/**/*.ts'), // Add other paths where IPC handlers might be defined ]);packages/frontend/apps/electron/src/entries/preload/ipc-events.ts (2)
16-25: Consider adding error handling to the IPC event listener.The current implementation doesn't handle potential errors from IPC callbacks.
Wrap the callback execution in a try-catch block:
ipcRenderer.on( AFFINE_IPC_EVENT_CHANNEL_NAME, (_event: Electron.IpcRendererEvent, channel: string, ...args: any[]) => { // Get all callbacks registered for this specific channel const callbacks = mainListenersMap.get(channel); if (callbacks) { - callbacks.forEach(callback => callback(...args)); + callbacks.forEach(callback => { + try { + callback(...args); + } catch (error) { + console.error(`Error in IPC event handler for channel '${channel}':`, error); + } + }); } } );
113-129: Improve warning logs for non-array eventNames.These warning logs are good for debuggability, but consider adding additional context or actions to make them more actionable.
Enhance the warning message to provide more actionable information:
eventsMeta.main?.forEach(([scope, eventNames]) => { if (!Array.isArray(eventNames)) { console.warn( - `[Preload Events] Expected eventNames to be an array for main scope '${scope}', but got:`, + `[Preload Events] Expected eventNames to be an array for main scope '${scope}', but got ${typeof eventNames}. This may indicate an issue with the type generation script or metadata format:`, eventNames ); } }); eventsMeta.helper?.forEach(([scope, eventNames]) => { if (!Array.isArray(eventNames)) { console.warn( - `[Preload Events] Expected eventNames to be an array for helper scope '${scope}', but got:`, + `[Preload Events] Expected eventNames to be an array for helper scope '${scope}', but got ${typeof eventNames}. This may indicate an issue with the type generation script or metadata format:`, eventNames ); } });packages/frontend/apps/electron/src/shared/type.ts (1)
20-20: Fix empty interfaceThe
HelperToMaininterface is empty, which is flagged by static analysis. Empty interfaces are equivalent to{}and don't provide any meaningful type constraints.-export interface HelperToMain {} +export type HelperToMain = Record<string, never>;🧰 Tools
🪛 Biome (1.9.4)
[error] 20-20: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
packages/frontend/apps/electron/src/entries/helper/helper-bootstrap.service.ts (1)
56-66: Logging wrapper risks leaking large/complex objects & slows hot-paths
handlerWithLogstringifies every non-function/non-object argument and always callsperformance.now().
For very frequent IPC calls this adds measurable overhead and the implicittoString()may emit PII or huge blobs.- const result = await handler(...args); - this.logger.debug( - `${channel}`, - 'async-api', - `${args.filter(arg => typeof arg !== 'function' && typeof arg !== 'object')}` - + ` - ${(performance.now() - start).toFixed(2)} ms` - ); + const result = await handler(...args); + if (this.logger.isDebugEnabled?.()) { + this.logger.debug( + `${channel} – ${(performance.now() - start).toFixed(2)} ms`, + this.context + ); + }Consider:
- Logging only when
Loggeris in DEBUG mode.- Truncating / hashing long arguments.
- Replacing
performance.now()withDate.now()if high-res timing is not required.packages/frontend/apps/electron/src/entries/main/helper-process/helper-process.service.ts (3)
109-116: Use optional-chaining to simplify & avoid double evaluation ofkill()
Callingthis.utilityProcess.kill()inside theifclause both checks and kills the process in one go; ifkill()returnsfalsethe process is still dead but you’ll log that it was “not running”. Optional chaining expresses the intent more clearly and avoids any side-effects during the check.-if (this.utilityProcess && this.utilityProcess.kill()) { - this.logger.log('Helper process killed.'); -} else { - this.logger.log('Helper process was not running or already killed.'); -} +if (this.utilityProcess?.kill()) { + this.logger.log('Helper process killed.'); +} else { + this.logger.log('Helper process was not running or already killed.'); +}This also satisfies the Biome lint warning reported by CI.
🧰 Tools
🪛 Biome (1.9.4)
[error] 111-111: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
68-72: Hard-coded inspector port collides when multiple helpers are spawned
--inspect=40895is fixed. If two dev instances start (e.g., parallel test runs) the second process will fail to bind.Use
--inspect=0to request a random free port or derive a port fromprocess.pid:-execArgv: isDev ? ['--inspect=40895'] : [], +execArgv: isDev ? ['--inspect=0'] : [],Expose the selected port via
helperProcess.debugPortif you need to attach later.
93-103:readyPromise is replaced on exit – callers may hang forever
If consumers hold a reference to the originalreadypromise and the helper crashes, they will wait indefinitely because the promise is never settled again. Consider switching to an RxJSBehaviorSubject/ReplaySubjector expose a methodawaitReady()that always returns the current promise.At minimum, document that callers must always read
.readyafter each failure rather than caching it.packages/frontend/apps/electron/scripts/ipc-generator/handlers-parser.ts (1)
129-133:IpcMainInvokeEventfiltering may miss aliases or re-exportsThe current check
if (index === 0 && paramType.includes('IpcMainInvokeEvent')) return;fails if the user imports the event type under an alias or uses a re-export.
A more reliable way is to inspect theSymbolor compare against the full
declaration name.packages/frontend/apps/electron/src/entries/main/storage/json-file.ts (2)
37-42: Emit current value on subscription to avoid “cold start” gaps
watch/watchAllonly emit after the first mutation, meaning a subscriber
cannot know the current persisted value until something changes.- return new Observable<T | undefined>(subscriber => { - const sub = (p: any) => subscriber.next(p); + return new Observable<T | undefined>(subscriber => { + // emit current value immediately + subscriber.next(this.data[key] as T | undefined); + const sub = (p: any) => subscriber.next(p);Same pattern can be applied to
watchAll.
64-75: Consider debouncing outside the hot path
set,del, andclearall synchronously loop over subscription sets before
returning. With many subscribers this can become a bottleneck. Debouncing the
saveeffect helps disk I/O but not in-memory fan-out.If storage mutations are frequent, wrapping the subscription notification in
queueMicrotaskorsetImmediatekeeps the method non-blocking.packages/frontend/apps/electron/src/logger/index.ts (1)
48-53: Structured logging for objects & errors
JSON.stringifywill silently drop circular references and loses stack traces
forErrorobjects. Usingutil.inspect(depth-infinite, colors false) or
electron-log’s own object handling will preserve more information.- if (typeof v === 'object') { - return JSON.stringify(v); - } + if (typeof v === 'object') { + return require('node:util').inspect(v, { depth: null, colors: false }); + }packages/frontend/apps/electron/src/entries/helper/types.ts (1)
6-9: Preferunknown& generics overanyfor stronger type safetyThe helper IPC types currently expose
(...args: any[]) => any, which forfeits compile-time checking.-export type NamespacedHandlers = Record< - string, - Record<string, (...args: any[]) => any> ->; +export type NamespacedHandlers = Record< + string, + Record<string, (...args: unknown[]) => unknown> +>; ... -export type EventRegistration = (cb: (...args: any[]) => void) => () => void; +export type EventRegistration = <T = unknown>( + cb: (...args: T[]) => void +) => () => void;This tiny change propagates safer types across your generated RPC interfaces without affecting runtime behaviour.
Also applies to: 15-23, 28-45
packages/frontend/apps/electron/src/ipc/readme.md (1)
37-41: Minor grammar polish“Unlike the main process, it doesn't include the
IpcMainInitializerService…”Small article insertion improves readability. Feel free to scan the doc for similar omissions flagged by LanguageTool.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~39-~39: You might be missing the article “the” here.
Context: ...ElectronIpcModule.forHelper()- Unlike main process, it doesn't include theIpcMai...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[grammar] ~39-~39: Please verify. Did you mean “It” or “the IT” (= information technology)?
Context: ...Unlike main process, it doesn't include theIpcMainInitializerService- It still uses theIpcScannerto find dec...(THE_IT)
packages/frontend/apps/electron/src/ipc/ipc.module.ts (1)
61-81: Use optional chaining for cleaner, safer access to nested webContents
win.contentView && win.contentView.childrencan be compressed and guarded in one go:- if (win.contentView && win.contentView.children) { - win.contentView.children.forEach(child => { - if ( - child instanceof WebContentsView && - child.webContents && - !child.webContents.isDestroyed() - ) { - child.webContents.send( - AFFINE_IPC_EVENT_CHANNEL_NAME, - channel, - ...args - ); - } - }); - } + win.contentView?.children?.forEach(child => { + if (child instanceof WebContentsView && !child.webContents?.isDestroyed()) { + child.webContents.send( + AFFINE_IPC_EVENT_CHANNEL_NAME, + channel, + ...args + ); + } + });Besides readability, this shields against potential
undefinedaccess if Electron ever changes internals.🧰 Tools
🪛 Biome (1.9.4)
[error] 66-66: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/frontend/apps/electron/scripts/ipc-generator/utils.ts (2)
7-10: MakedetermineEntrycross-platform-safe
path.includes('src/entries/helper')works on *nix paths but fails on Windows where the separator is\. Prefer usingpath.posix/path.win32normalisation (or simplypath.sep+replaceAll('\\', '/')) before the check, to avoid incorrectly classifying helper files on Windows CI/dev machines.+import path from 'node:path'; + export const determineEntry = (rawPath: string): Entry => { - if (path.includes('src/entries/helper')) return 'helper'; - return 'main'; + const normalized = rawPath.split(path.sep).join('/'); + return normalized.includes('src/entries/helper') ? 'helper' : 'main'; };
64-65: Edge-case: event name capitalisation
on${eventName.charAt(0).toUpperCase() + eventName.slice(1)}blindly upper-cases the first UTF-16 code unit.
IfeventNamealready starts with an uppercase letter or contains non-latin chars the result may be surprising.
Consider using a dedicated util such aslodash.upperFirstor leaving the original casing unchanged to avoid accidental renaming.packages/frontend/apps/electron/scripts/ipc-generator/types.ts (1)
34-37: Remove seemingly unusedCollectedEventInfoForMeta
CollectedEventInfoForMetais currently not referenced anywhere in the provided code (only theForTypesvariant is used).
Keeping dead types increases cognitive load. If it’s truly unneeded, delete it; otherwise add usage or a comment clarifying its purpose.packages/frontend/apps/electron/scripts/ipc-generator/events-parser.ts (3)
37-48: Simplify nested null checks with optional chaining
The scope-extraction block is verbose and flagged by Biome. Optional chaining makes the intent clearer and trims noise.- let scopeValue: string | undefined; - const scopeProperty = optionsArg.getProperty('scope'); - if (scopeProperty && Node.isPropertyAssignment(scopeProperty)) { - const initializer = scopeProperty.getInitializer(); - if (initializer) { - if (Node.isStringLiteral(initializer)) - scopeValue = initializer.getLiteralValue(); - else if (Node.isPropertyAccessExpression(initializer)) { - const type = initializer.getType(); - if (type.isStringLiteral()) - scopeValue = type.getLiteralValue() as string; - else scopeValue = initializer.getNameNode().getText(); - } - } - } + const scopeInitializer = optionsArg + .getProperty('scope') + ?.asKind?.(tsMorph.SyntaxKind.PropertyAssignment) + ?.getInitializer(); + + const scopeValue = + Node.isStringLiteral(scopeInitializer) + ? scopeInitializer.getLiteralValue() + : Node.isPropertyAccessExpression(scopeInitializer) + ? (scopeInitializer.getType().isStringLiteral() + ? scopeInitializer.getType().getLiteralValue() + : scopeInitializer.getNameNode().getText()) + : undefined;
119-123: Preferunknownoveranyfor unresolved payloads
Falling back to the broadanytype forfeits all type-safety.
Usingunknownforces consumers to narrow the type explicitly and prevents accidental misuse.- if (payloadType === 'any[]') { - payloadType = 'any'; + if (payloadType === 'any[]') { + payloadType = 'unknown'; }
1-1: Name shadowing with globalNode
Importing{ Node }fromts-morphshadows the globalNodetype from the DOM lib.
While not harmful in a Node.js environment, it can confuse IDE quick-info & future maintainers.
Consider aliasing:import { Node as TsNode } from 'ts-morph';and adjusting usages accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (71)
oxlint.json(1 hunks)packages/frontend/apps/electron/package.json(3 hunks)packages/frontend/apps/electron/scripts/common.ts(3 hunks)packages/frontend/apps/electron/scripts/dev.ts(2 hunks)packages/frontend/apps/electron/scripts/generate-types.ts(1 hunks)packages/frontend/apps/electron/scripts/ipc-generator/events-parser.ts(1 hunks)packages/frontend/apps/electron/scripts/ipc-generator/handlers-parser.ts(1 hunks)packages/frontend/apps/electron/scripts/ipc-generator/types.ts(1 hunks)packages/frontend/apps/electron/scripts/ipc-generator/utils.ts(1 hunks)packages/frontend/apps/electron/src-old/shared/type.ts(1 hunks)packages/frontend/apps/electron/src-old/shared/utils.ts(1 hunks)packages/frontend/apps/electron/src/entries/helper/app.module.ts(1 hunks)packages/frontend/apps/electron/src/entries/helper/bootstrap.ts(1 hunks)packages/frontend/apps/electron/src/entries/helper/dialog/dialog-handler.service.ts(1 hunks)packages/frontend/apps/electron/src/entries/helper/dialog/dialog.module.ts(1 hunks)packages/frontend/apps/electron/src/entries/helper/helper-bootstrap.service.ts(1 hunks)packages/frontend/apps/electron/src/entries/helper/index.ts(1 hunks)packages/frontend/apps/electron/src/entries/helper/logger/index.ts(1 hunks)packages/frontend/apps/electron/src/entries/helper/main-rpc.ts(1 hunks)packages/frontend/apps/electron/src/entries/helper/shared/types.ts(1 hunks)packages/frontend/apps/electron/src/entries/helper/types.ts(1 hunks)packages/frontend/apps/electron/src/entries/helper/workspace/workspace-events.service.ts(1 hunks)packages/frontend/apps/electron/src/entries/helper/workspace/workspace.module.ts(1 hunks)packages/frontend/apps/electron/src/entries/main/app.module.ts(1 hunks)packages/frontend/apps/electron/src/entries/main/bootstrap.ts(1 hunks)packages/frontend/apps/electron/src/entries/main/helper-process/helper-process.module.ts(1 hunks)packages/frontend/apps/electron/src/entries/main/helper-process/helper-process.service.ts(1 hunks)packages/frontend/apps/electron/src/entries/main/helper-process/index.ts(1 hunks)packages/frontend/apps/electron/src/entries/main/index.ts(1 hunks)packages/frontend/apps/electron/src/entries/main/logger/index.ts(1 hunks)packages/frontend/apps/electron/src/entries/main/storage/events.ts(1 hunks)packages/frontend/apps/electron/src/entries/main/storage/handlers.ts(1 hunks)packages/frontend/apps/electron/src/entries/main/storage/index.ts(1 hunks)packages/frontend/apps/electron/src/entries/main/storage/json-file.ts(1 hunks)packages/frontend/apps/electron/src/entries/main/storage/persist.ts(1 hunks)packages/frontend/apps/electron/src/entries/main/storage/storage.ts(1 hunks)packages/frontend/apps/electron/src/entries/main/windows/context-menu.service.ts(1 hunks)packages/frontend/apps/electron/src/entries/main/windows/custom-theme-window.service.ts(1 hunks)packages/frontend/apps/electron/src/entries/main/windows/events.ts(1 hunks)packages/frontend/apps/electron/src/entries/main/windows/index.ts(1 hunks)packages/frontend/apps/electron/src/entries/main/windows/main-window.service.ts(1 hunks)packages/frontend/apps/electron/src/entries/main/windows/popup.service.ts(1 hunks)packages/frontend/apps/electron/src/entries/main/windows/tab-views.service.ts(1 hunks)packages/frontend/apps/electron/src/entries/main/windows/utils.ts(1 hunks)packages/frontend/apps/electron/src/entries/main/windows/windows.module.ts(1 hunks)packages/frontend/apps/electron/src/entries/main/windows/windows.service.ts(1 hunks)packages/frontend/apps/electron/src/entries/preload/.gitignore(1 hunks)packages/frontend/apps/electron/src/entries/preload/api-info.ts(1 hunks)packages/frontend/apps/electron/src/entries/preload/helper-rpc.ts(1 hunks)packages/frontend/apps/electron/src/entries/preload/index.ts(1 hunks)packages/frontend/apps/electron/src/entries/preload/ipc-events.ts(1 hunks)packages/frontend/apps/electron/src/entries/preload/ipc-handlers.ts(1 hunks)packages/frontend/apps/electron/src/ipc/constant.ts(1 hunks)packages/frontend/apps/electron/src/ipc/index.ts(1 hunks)packages/frontend/apps/electron/src/ipc/ipc-event.ts(1 hunks)packages/frontend/apps/electron/src/ipc/ipc-handle.ts(1 hunks)packages/frontend/apps/electron/src/ipc/ipc-scanner.ts(1 hunks)packages/frontend/apps/electron/src/ipc/ipc.module.ts(1 hunks)packages/frontend/apps/electron/src/ipc/readme.md(1 hunks)packages/frontend/apps/electron/src/logger/index.ts(1 hunks)packages/frontend/apps/electron/src/shared/constants.ts(1 hunks)packages/frontend/apps/electron/src/shared/type.ts(1 hunks)packages/frontend/apps/electron/src/shared/utils.ts(0 hunks)packages/frontend/apps/electron/tsconfig.json(1 hunks)packages/frontend/core/src/desktop/dialogs/setting/general-setting/meetings/index.tsx(1 hunks)packages/frontend/core/src/modules/editor-setting/services/spell-check-setting.ts(1 hunks)packages/frontend/core/src/modules/editor-setting/services/tray-settings.ts(1 hunks)packages/frontend/core/src/modules/media/services/meeting-settings.ts(1 hunks)packages/frontend/electron-api/src/index.ts(1 hunks)packages/frontend/electron-api/src/ipc-api-types.gen.ts(1 hunks)packages/frontend/electron-api/src/ipc-event-types.gen.ts(1 hunks)
💤 Files with no reviewable changes (1)
- packages/frontend/apps/electron/src/shared/utils.ts
🧰 Additional context used
🧬 Code Graph Analysis (17)
packages/frontend/apps/electron/src/entries/main/app.module.ts (1)
packages/frontend/apps/electron/src/ipc/ipc.module.ts (1)
ElectronIpcModule(105-123)
packages/frontend/apps/electron/src/entries/main/logger/index.ts (1)
packages/frontend/apps/electron/src/logger/index.ts (1)
createLoggerService(85-87)
packages/frontend/apps/electron/src/entries/preload/index.ts (2)
packages/frontend/apps/electron/src/entries/preload/ipc-handlers.ts (1)
exposedApis(57-60)packages/frontend/apps/electron/src/entries/preload/ipc-events.ts (1)
exposedEvents(107-110)
packages/frontend/apps/electron/src/entries/main/windows/events.ts (1)
packages/frontend/apps/electron/src/ipc/ipc-event.ts (1)
IpcEvent(14-58)
packages/frontend/apps/electron/src/entries/preload/helper-rpc.ts (1)
packages/frontend/apps/electron/src/ipc/constant.ts (1)
AFFINE_HELPER_CONNECT_CHANNEL_NAME(15-15)
packages/frontend/apps/electron/src/entries/preload/ipc-handlers.ts (2)
packages/frontend/apps/electron/src/ipc/constant.ts (1)
AFFINE_IPC_API_CHANNEL_NAME(11-11)packages/frontend/apps/electron/src/entries/preload/helper-rpc.ts (1)
helperRpc(51-54)
packages/frontend/apps/electron/src/entries/preload/ipc-events.ts (3)
packages/frontend/apps/electron/src/ipc/constant.ts (1)
AFFINE_IPC_EVENT_CHANNEL_NAME(12-12)packages/frontend/apps/android/App/app/src/main/java/app/affine/pro/service/GraphQLService.kt (1)
subscription(105-110)packages/frontend/apps/electron/src/entries/preload/helper-rpc.ts (1)
helperEvents$(35-35)
packages/frontend/apps/electron/src/entries/helper/dialog/dialog-handler.service.ts (1)
packages/frontend/apps/electron/src/ipc/ipc-handle.ts (1)
IpcHandle(14-56)
packages/frontend/apps/electron/src/entries/helper/app.module.ts (1)
packages/frontend/apps/electron/src/ipc/ipc.module.ts (1)
ElectronIpcModule(105-123)
packages/frontend/electron-api/src/index.ts (3)
packages/frontend/electron-api/src/ipc-event-types.gen.ts (1)
ElectronEvents(2-12)packages/frontend/electron-api/src/ipc-api-types.gen.ts (1)
ElectronApis(6-20)packages/frontend/apps/electron/src-old/preload/shared-storage.ts (1)
SharedStorage(98-98)
packages/frontend/apps/electron/src/ipc/ipc-scanner.ts (2)
packages/frontend/apps/electron/src/ipc/ipc-handle.ts (1)
IPC_HANDLE_META_KEY(5-5)packages/frontend/apps/electron/src/ipc/ipc-event.ts (1)
IPC_EVENT_META_KEY(3-3)
packages/frontend/apps/electron/src/entries/helper/logger/index.ts (1)
packages/frontend/apps/electron/src/logger/index.ts (1)
createLoggerService(85-87)
packages/frontend/apps/electron/scripts/ipc-generator/utils.ts (1)
packages/frontend/apps/electron/scripts/ipc-generator/types.ts (3)
Entry(23-23)CollectedApisMap(32-32)CollectedEventsMap(42-42)
packages/frontend/apps/electron/scripts/ipc-generator/handlers-parser.ts (2)
packages/frontend/apps/electron/scripts/ipc-generator/types.ts (2)
ParsedDecoratorInfo(5-10)CollectedApisMap(32-32)packages/frontend/apps/electron/scripts/ipc-generator/utils.ts (1)
determineEntry(7-10)
packages/frontend/apps/electron/scripts/ipc-generator/events-parser.ts (2)
packages/frontend/apps/electron/scripts/ipc-generator/types.ts (2)
ParsedEventInfo(12-21)CollectedEventsMap(42-42)packages/frontend/apps/electron/scripts/ipc-generator/utils.ts (1)
determineEntry(7-10)
packages/frontend/apps/electron/src/entries/main/windows/windows.service.ts (1)
packages/frontend/apps/electron/src/ipc/ipc-handle.ts (1)
IpcHandle(14-56)
packages/frontend/apps/electron/src/entries/main/helper-process/helper-process.service.ts (1)
packages/frontend/apps/electron/src/ipc/constant.ts (2)
AFFINE_RENDERER_CONNECT_CHANNEL_NAME(14-14)AFFINE_HELPER_CONNECT_CHANNEL_NAME(15-15)
🪛 Biome (1.9.4)
packages/frontend/apps/electron/src/entries/main/windows/popup.service.ts
[error] 5-5: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
packages/frontend/apps/electron/src/entries/main/windows/tab-views.service.ts
[error] 5-5: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
packages/frontend/apps/electron/src/entries/main/windows/context-menu.service.ts
[error] 5-5: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
packages/frontend/apps/electron/src/entries/main/windows/custom-theme-window.service.ts
[error] 5-5: This constructor is unnecessary.
Unsafe fix: Remove the unnecessary constructor.
(lint/complexity/noUselessConstructor)
packages/frontend/apps/electron/src/shared/type.ts
[error] 20-20: An empty interface is equivalent to {}.
Safe fix: Use a type alias instead.
(lint/suspicious/noEmptyInterface)
packages/frontend/apps/electron/src/ipc/ipc.module.ts
[error] 66-66: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 105-123: Avoid classes that contain only static members.
Prefer using simple functions instead of classes with only static members.
(lint/complexity/noStaticOnlyClass)
packages/frontend/apps/electron/scripts/ipc-generator/events-parser.ts
[error] 83-83: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
[error] 85-85: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/frontend/apps/electron/src/entries/main/helper-process/helper-process.service.ts
[error] 111-111: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🪛 GitHub Actions: Build & Test
packages/frontend/apps/electron/src/entries/main/storage/handlers.ts
[error] 1-1: TypeScript error TS2307: Cannot find module '../type' or its corresponding type declarations.
packages/frontend/apps/electron/src/entries/preload/ipc-handlers.ts
[error] 5-5: Could not resolve module './ipc-meta.gen' imported at line 5.
packages/frontend/apps/electron/src/entries/preload/ipc-events.ts
[error] 6-6: Could not resolve module './ipc-meta.gen' imported at line 6.
🪛 LanguageTool
packages/frontend/apps/electron/src/ipc/readme.md
[uncategorized] ~39-~39: You might be missing the article “the” here.
Context: ...ElectronIpcModule.forHelper()- Unlike main process, it doesn't include theIpcMai...
(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[grammar] ~39-~39: Please verify. Did you mean “It” or “the IT” (= information technology)?
Context: ...Unlike main process, it doesn't include the IpcMainInitializerService - It still uses the IpcScanner to find dec...
(THE_IT)
[uncategorized] ~54-~54: A determiner appears to be missing. Consider inserting it.
Context: ...Main-to-Helper RPC Communication:* - Helper process does not have access to Electro...
(AI_EN_LECTOR_MISSING_DETERMINER)
[uncategorized] ~80-~80: A determiner appears to be missing. Consider inserting it.
Context: ... }, }); } } ``` - Main process connects to helper via `HelperP...
(AI_EN_LECTOR_MISSING_DETERMINER)
[uncategorized] ~106-~106: Loose punctuation mark.
Context: ...pe Files:** - ipc-api-types.gen.ts: Interface for API methods (`ElectronApi...
(UNLIKELY_OPENING_PUNCTUATION)
🔇 Additional comments (69)
packages/frontend/core/src/desktop/dialogs/setting/general-setting/meetings/index.tsx (1)
15-15:Details
✅ Verification successful
Verify the new import path resolves correctly
Ensure the file atapps/electron/src-old/main/shared-state-schema.ts(or.d.ts) still exists and that your build setup can resolve this path. You can run:
🏁 Script executed:
#!/usr/bin/env bash # Verify presence of the shared-state-schema file fd shared-state-schema apps/electron/src-old/main/shared-state-schema.*Length of output: 211
🏁 Script executed:
#!/usr/bin/env bash # Search for any occurrences of shared-state-schema files in the repo fd shared-state-schema . # Fallback: list any files importing or declaring shared-state-schema rg -l "shared-state-schema" --glob "*.ts" --glob "*.d.ts"Length of output: 685
Import path verified
The filepackages/frontend/apps/electron/src-old/main/shared-state-schema.tsexists at the expected location, so the relative import in
packages/frontend/core/src/desktop/dialogs/setting/general-setting/meetings/index.tsxresolves correctly.No further action required.
packages/frontend/apps/electron/src/entries/main/helper-process/index.ts (1)
1-2: Consolidate helper-process exports.This barrel file correctly re-exports the module and service, providing a single entry point for the helper process. Imports elsewhere in the application will be cleaner and more maintainable.
packages/frontend/apps/electron/src/entries/main/storage/index.ts (1)
1-1: Consolidate storage handlers exports.The new index file cleanly aggregates exports from
handlers, making it easier to import storage-related functionality. No issues detected.packages/frontend/apps/electron/src/entries/main/windows/index.ts (1)
1-4: Consolidate window management exports.This barrel file unifies exports from multiple window-related modules and services, simplifying downstream imports and aligning with the NestJS modular structure.
packages/frontend/apps/electron/tsconfig.json (1)
4-9: Enable decorators and source maps for NestJS integration.Enabling
experimentalDecorators,emitDecoratorMetadata, andsourceMapis essential for NestJS and debugging. ThetsBuildInfoFilepath is correctly pointed to thedistdirectory to speed up incremental builds.packages/frontend/apps/electron/src/entries/helper/index.ts (1)
1-6: Well-structured entry point with proper error handling.This entry file follows the standard pattern for Node.js applications, with appropriate error handling that logs exceptions and exits cleanly on bootstrap failures.
packages/frontend/apps/electron/src/entries/main/index.ts (1)
1-7: Well-structured entry point with proper error handling using a dedicated logger.This main process entry point follows best practices by using a dedicated logger instead of console.error, which provides better production logging capabilities. The bootstrap error handling ensures the application exits cleanly on initialization failures.
oxlint.json (1)
199-202: Appropriate linting configuration for NestJS integration.Disabling the
consistent-type-importsrule for the electron app's TypeScript files is necessary to accommodate NestJS decorators and dependency injection patterns, which often require importing both types and values from the same module.packages/frontend/apps/electron/src/entries/helper/dialog/dialog.module.ts (1)
1-12: Well-structured NestJS moduleThe module is properly configured with providers and exports, following NestJS conventions. The JSDoc comment clearly explains the module's purpose.
packages/frontend/apps/electron/src/entries/helper/workspace/workspace.module.ts (1)
1-12: Module implementation follows NestJS best practicesThe
WorkspaceModuleis correctly implemented as a standard NestJS module with proper provider and export configurations. The module has a clear single responsibility and includes helpful documentation.packages/frontend/apps/electron/src/ipc/index.ts (1)
1-5: Well-structured barrel file for IPC exportsThis barrel file pattern is an effective way to simplify imports for consumers of the IPC system. It centralizes access to all IPC-related functionality through a single import, which improves code organization.
packages/frontend/apps/electron/src/entries/main/app.module.ts (1)
1-16: AppModule correctly serves as composition root for main processThe module structure properly organizes the main process dependencies by importing all necessary modules: window management, logging, IPC handling, and helper process management. The use of
ElectronIpcModule.forMain()appropriately configures IPC for the main process context.packages/frontend/apps/electron/src/entries/helper/logger/index.ts (1)
1-18: Logger module implementation follows NestJS best practicesThe
LoggerModuleis correctly implemented as a global module with transient scope, making the logger available throughout the application. The approach of creating a scoped logger service is appropriate for the helper process.packages/frontend/apps/electron/src/entries/main/windows/windows.module.ts (1)
1-13: Well-structured NestJS module with clear responsibilities.The
WindowsModulefollows NestJS best practices by clearly defining its providers, exports, and dependencies. The module has a focused responsibility of grouping window management services together.packages/frontend/apps/electron/src/entries/helper/bootstrap.ts (1)
6-21: Clean bootstrap implementation with proper error handling.The bootstrap function correctly sets up the NestJS application context and includes appropriate shutdown handling. The comments provide clarity on where the parentPort message handling occurs.
packages/frontend/apps/electron/src/entries/main/helper-process/helper-process.module.ts (1)
1-9: Focused module with single responsibility.This module follows the single responsibility principle by providing and exporting just the
HelperProcessService. The implementation follows standard NestJS module structure and conventions.packages/frontend/apps/electron/src/entries/preload/index.ts (1)
1-9: Secure implementation using contextBridge.This preload script correctly uses Electron's contextBridge to securely expose APIs and events to the renderer process, following Electron security best practices. The exposed objects are appropriately namespaced with double underscores to avoid conflicts.
packages/frontend/apps/electron/src/entries/main/logger/index.ts (2)
5-5: Singleton logger instance createdThe logger is created with the 'main' scope, allowing for proper context in log messages.
7-18: Well-structured NestJS module following best practicesThe module is correctly set up as a global module with a transient-scoped logger provider, making it available throughout the application without explicit imports. This follows NestJS best practices for shared services.
packages/frontend/apps/electron/src/entries/main/storage/handlers.ts (1)
7-10: Implementation uses type-safe satisfies operatorThe implementation correctly uses the
satisfiesoperator to ensure type compatibility with theNamespaceHandlersinterface while maintaining the exact return types of the methods.packages/frontend/apps/electron/src/entries/main/storage/storage.ts (1)
7-9: Good use of platform-specific pathsThe implementation correctly uses Electron's
app.getPath('userData')to get the platform-specific user data directory, ensuring cross-platform compatibility.packages/frontend/apps/electron/src/entries/main/windows/events.ts (1)
6-23: Good implementation of window events with NestJS and RxJSThe
WindowEventsclass is well-structured, using RxJS Subjects with proper IPC decorators for cross-process communication. The implementation follows NestJS patterns correctly.packages/frontend/apps/electron/src/entries/helper/app.module.ts (1)
10-24: Well-organized NestJS module structureThe
AppModulefollows NestJS best practices with clear module imports and provider definitions. The organization with feature modules (WorkspaceModule,DialogModule) and infrastructure modules (LoggerModule,ElectronIpcModule) is appropriate.packages/frontend/electron-api/src/ipc-api-types.gen.ts (1)
1-21: Well-structured type definitions for Electron IPC interfaceThis auto-generated interface provides a clear contract for IPC communication between processes. The interface defines dialog methods with proper parameter types and return values, alongside a UI method for window management.
packages/frontend/apps/electron/src/entries/main/storage/events.ts (1)
4-25: Clean implementation of subscription-based event patternThe storage events implementation follows good reactive programming practices:
- Each event handler properly returns an unsubscribe function to prevent memory leaks
- The type definition ensures type safety with the
satisfiesoperator- The pattern is consistent between global state and cache events
packages/frontend/apps/electron/src/entries/preload/ipc-handlers.ts (2)
7-23: Well-designed factory pattern for IPC handlersThe factory functions provide a clean abstraction for creating IPC handlers:
- Main process handlers use the standard Electron IPC system
- Helper handlers include proper error handling and logging
- Both follow a consistent pattern for method invocation
27-54: Robust dynamic API constructionThe dynamic construction of API objects is well-implemented with:
- Defensive programming (checking if arrays exist before processing)
- Filtering to ensure only valid method names are processed
- Clear naming that helps with understanding the code flow
packages/frontend/apps/electron/scripts/common.ts (4)
8-8: Good addition of TypeScript compiler plugin for NestJS supportAdding the esbuild-plugin-tsc is necessary for proper NestJS decorator support, as the comment correctly indicates.
42-47: Proper configuration of TypeScript compiler pluginThe plugin is correctly configured with the appropriate tsconfig path. This ensures that NestJS decorators will work properly with esbuild.
87-89: Improved entry point organizationThe entry points have been reorganized into a clearer structure under the entries directory, separating main, preload, and helper processes.
96-110: Comprehensive external dependencies listThe external dependencies list has been expanded to include all necessary NestJS-related packages. This prevents bundling these packages with the application, which is appropriate since they'll be available at runtime.
packages/frontend/apps/electron/src/entries/main/windows/windows.service.ts (1)
1-50: Well-structured WindowsService implementation with clear responsibilitiesThis service effectively manages application windows as the main "launcher" using NestJS dependency injection. The implementation follows good practices by:
- Properly implementing OnModuleInit to initialize after the app is ready
- Delegating window-specific operations to MainWindowService
- Using IPC decoration for inter-process communication
- Setting up appropriate event forwarding
The code is clean, well-organized, and follows separation of concerns principles.
packages/frontend/apps/electron/src/ipc/ipc-handle.ts (1)
1-56: Well-designed IPC handle decorator with thorough validationThis implementation of the
@IpcHandledecorator is robust and well-designed:
- It performs thorough validation of the input options
- It ensures the decorator is only applied to methods
- It properly constructs channel names with a consistent pattern
- It uses Reflect metadata for storing IPC channel information
The code is clean, maintainable, and provides clear error messages.
packages/frontend/apps/electron/src/entries/helper/main-rpc.ts (2)
1-9: Imports and type setup look good.The code correctly imports necessary dependencies from NestJS and async-call-rpc, and references the MainToHelper type from the shared types.
34-39: Module setup follows NestJS conventions.The Global decorator ensures the service is available throughout the application without needing to import the module in every module.
packages/frontend/apps/electron/src/entries/preload/helper-rpc.ts (2)
1-9: Imports look appropriate.The code imports all necessary dependencies for establishing communication between the renderer and helper processes.
35-40: Event handling implementation is clean.The Subject and server implementation creates a clean way to handle events from the renderer to the helper.
packages/frontend/apps/electron/scripts/dev.ts (3)
77-79: Good improvement to build state tracking.Adding hasBuiltOnce flag and returning a proper Promise with resolve/reject improves build lifecycle management.
109-131: Improved error handling for build failures.The error handling for build failures is much better now, with proper rejection of the Promise on initial build failure.
137-148: Better logging and error propagation.The additional logging and explicit error propagation improves the developer experience and makes debugging easier.
packages/frontend/apps/electron/package.json (4)
26-26: Good integration of type generation in build process.Integrating the type generation script in the build process ensures that type definitions are always up-to-date.
33-33: Useful standalone type generation script.Adding a separate script for type generation is helpful for development and testing purposes.
49-51: NestJS dependencies added correctly.The NestJS dependencies are correctly added to support the new architecture.
69-69: Supporting dependencies for TypeScript and NestJS integration.The additional dependencies for TypeScript tooling, reflection metadata, and esbuild plugin are appropriate for the NestJS integration.
Also applies to: 75-75, 79-81
packages/frontend/apps/electron/src/entries/helper/workspace/workspace-events.service.ts (2)
1-23: Well-structured service with clear purpose and good dependency injectionThe
WorkspaceEventsServiceclass follows good NestJS practices with proper dependency injection and clear separation of concerns. The@IpcEventdecorator is used correctly to expose the RxJS Subject for IPC communication, with appropriate typing for the payload.
24-33: Well-implemented event emission with proper loggingThe
emitMetaChangemethod provides a clean API for emitting workspace metadata changes with appropriate logging. The debug message includes the workspace ID, which is helpful for troubleshooting.packages/frontend/apps/electron/src/ipc/constant.ts (2)
1-9: Well-defined IPC scopes with consistent namingThe
IpcScopeenum provides a clear and organized way to categorize different types of IPC communications. Using string enum values is appropriate for IPC channel names.
11-15: Consistent channel naming with clear purposeThe channel name constants follow a consistent naming convention with the
AFFINE_prefix. Each constant has a clear purpose within the IPC system.packages/frontend/apps/electron/src/ipc/ipc-scanner.ts (1)
1-14: Good use of NestJS discovery mechanismsThe
IpcScannerclass is well-designed to leverage NestJS's built-in discovery and metadata scanning capabilities. The dependency injection is set up correctly with the required services.packages/frontend/apps/electron/src/ipc/ipc-event.ts (2)
1-13: Clear metadata key and options interfaceThe
IPC_EVENT_META_KEYsymbol andIpcEventOptionsinterface are well-defined. The interface has good comments explaining the purpose of each property.
14-41: Comprehensive input validation in decoratorThe decorator function includes thorough validation of inputs with descriptive error messages that include the target class and property names, which will help with debugging.
packages/frontend/apps/electron/src/entries/main/bootstrap.ts (2)
11-51: Well-structured initialization logic.The
beforeReadyfunction handles essential Electron configuration steps in a logical sequence. Good use of sandboxing and secure defaults, with clear explanations for various configuration decisions (like host mapping for dev server and feature disabling).
53-67: Clean NestJS integration with Electron.The bootstrap function properly integrates NestJS with Electron's lifecycle. Good handling of application shutdown with the
before-quitevent listener and proper error handling for the context closure.packages/frontend/electron-api/src/index.ts (4)
1-2: Improved type organization with generated typesNicely refactored to use the new auto-generated types (
ElectronApisandElectronEvents) rather than manually importing specific types. This simplifies maintenance and ensures type consistency across the IPC system.
4-9: LGTM: AppInfo type defined inlineThe inline definition of
AppInfotype improves co-location of related code.
11-12: Simplified type systemGood refactoring here. Creating direct aliases to the generated types (
ClientEvents = ElectronEventsandClientHandlers = ElectronApis) reduces duplication and makes the type system more maintainable.
28-33:Details
❓ Verification inconclusive
Note the transition to src-old directories
The imports are now pointing to
src-olddirectories, indicating this refactoring is part of a transitional architecture. Ensure these imports are updated once the migration to the new NestJS structure is complete.
🏁 Script executed:
#!/bin/bash # Check for other references to src-old directories to identify remaining migration work rg -g "*.ts" "src-old" packages/frontend/Length of output: 821
Update
src-oldimports across frontend once migration is completeThe following files still reference the legacy
src-olddirectories. These should be updated to point at the new NestJS modules once the migration is finished:
- packages/frontend/electron-api/src/index.ts
- packages/frontend/core/src/modules/media/services/meeting-settings.ts
- packages/frontend/core/src/modules/editor-setting/services/tray-settings.ts
- packages/frontend/core/src/modules/editor-setting/services/spell-check-setting.ts
Action items:
- Track and replace all
src-oldimports with their new NestJS counterparts.- Consider creating a codemod or script to bulk-update these paths when the new structure is ready.
packages/frontend/apps/electron/src/shared/type.ts (4)
3-4: Good pattern for event registrationThe
MainEventRegistertype follows the standard pattern of returning a cleanup function, which is consistent with React and other modern event systems. This facilitates proper resource management.
5-12: Well-structured handler typesThe
IsomorphicHandlerandNamespaceHandlerstypes provide a clean abstraction for IPC handlers. The "isomorphic" naming suggests these can work consistently across different processes, which aligns well with Electron's architecture.
15-18: Clean metadata structureThe
ExposedMetainterface provides a consistent structure for exposing handler and event metadata, which is crucial for the type-safe IPC system.
22-29: Type-safe API selectionGood use of TypeScript's
Pickutility to create a restricted interface for main-to-helper communication, ensuring only the necessary Electron APIs are exposed.packages/frontend/apps/electron/src-old/shared/type.ts (1)
1-32: Architectural improvement in the new designThis file has been moved to
src-oldas part of the refactoring. The new architecture (inpackages/frontend/apps/electron/src/shared/type.ts) improves upon this by:
- Using more generic and reusable types
- Supporting a decorator-based, dynamic IPC registration system
- Moving constants like
AFFINE_API_CHANNEL_NAMEto more appropriate locationsThe changes align well with NestJS principles of modularity and dependency injection.
packages/frontend/apps/electron/src-old/shared/utils.ts (3)
5-19: Platform and time utilities moved to specialized modulesThese utility functions have been moved or refactored as part of the NestJS-style restructuring. This improves code organization by placing utilities in more specific, domain-focused modules rather than in a general utilities file.
21-46: IPC channel abstractionsThe
MessagePortLikeinterface andMessageEventChannelclass provide solid abstractions for message-based communication between processes. This implementation follows best practices for event-based channels with proper cleanup.
48-83: Utility functions relocated to specialized modulesThe path and equality utility functions have been refactored as part of the new architecture. This supports better separation of concerns and modularity, which aligns with NestJS architectural principles.
packages/frontend/apps/electron/src/entries/helper/helper-bootstrap.service.ts (1)
27-33:Details
❌ Incorrect review comment
Verify usage of
process.parentPort– may be undefined in Node /Electron worker contexts
In a plain Node worker you usually importparentPortfrom thenode:worker_threadsmodule.
Electron’sUtilityProcessdoes exposeprocess.parentPort, but only after Electron 24. If the helper is launched with an older runtime (or unit-tests run under plain Node) this check will fail and the bootstrap will silently abort.import { parentPort } from 'node:worker_threads'; if (!parentPort) { ... }Please confirm the target Electron version guarantees the presence of
process.parentPort; otherwise import fromworker_threadsor feature-detect with optional chaining to avoid an early return that disables the helper entirely.
🏁 Script executed:
#!/bin/bash # Search for Electron dependency versions across all package.json files rg -n '"electron":' --glob '*package.json'Length of output: 221
Ignore import-from-worker_threads suggestion
Our project is pinned to Electron ^36.0.0 (well above v24), soprocess.parentPortis natively exposed in UtilityProcess workers. The existing guard correctly aborts when not running in a worker and no changes are needed.Likely an incorrect or invalid review comment.
packages/frontend/apps/electron/src/logger/index.ts (1)
41-47: Context parameter detection differs from NestJS contractNestJS’s
LoggerServicesignature islog(message, ...optionalParams)where
the first optional param is the context string, followed by rest params.
Treating the last param as context may reorder messages unexpectedly.Verify the intended contract or switch to
optionalParams[0]for parity with
the core logger.packages/frontend/apps/electron/src/entries/main/windows/main-window.service.ts (1)
99-101: Hard-coded URL may be a placeholder
loadURL('https://google.com')looks like debugging code. If forgotten, users will see an unexpected page in production.Please replace it with your actual renderer entry (e.g.
file://…/index.html) or read from a configuration constant/env variable.packages/frontend/apps/electron/src/ipc/ipc.module.ts (1)
34-59:ipcMain.handleleak on HMR / multiple initialisations
ipcMain.handleregisters once per channel. IfforMain()is imported twice (e.g., by reloading modules in development), subsequent initialisations will throwError: Attempted to register a second handler for channel.Consider:
- Guarding with
ipcMain.removeHandler(AFFINE_IPC_API_CHANNEL_NAME)beforehandle, or- Setting the module as
global: trueand importing only in the root module.
packages/frontend/core/src/modules/editor-setting/services/spell-check-setting.ts
Outdated
Show resolved
Hide resolved
packages/frontend/apps/electron/src/entries/main/storage/handlers.ts
Outdated
Show resolved
Hide resolved
packages/frontend/apps/electron/src/entries/main/storage/handlers.ts
Outdated
Show resolved
Hide resolved
packages/frontend/apps/electron/src/entries/main/storage/storage.ts
Outdated
Show resolved
Hide resolved
packages/frontend/apps/electron/src/entries/main/windows/main-window.service.ts
Show resolved
Hide resolved
packages/frontend/apps/electron/src/entries/main/windows/main-window.service.ts
Show resolved
Hide resolved
packages/frontend/apps/electron/scripts/ipc-generator/events-parser.ts
Outdated
Show resolved
Hide resolved
pengx17
force-pushed
the
xp/04-24-refactor_electron_nestjsfy
branch
7 times, most recently
from
7b6f823 to
7a38f7d
Compare
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
pengx17
force-pushed
the
xp/04-24-refactor_electron_nestjsfy
branch
10 times, most recently
from
c37779a to
2b4b760
Compare
pengx17
force-pushed
the
xp/04-24-refactor_electron_nestjsfy
branch
4 times, most recently
from
5bceee5 to
68a7077
Compare
pengx17
force-pushed
the
xp/04-24-refactor_electron_nestjsfy
branch
from
68a7077 to
29d43dd
Compare
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
pengx17
force-pushed
the
xp/04-24-refactor_electron_nestjsfy
branch
2 times, most recently
from
842dea8 to
bf829df
Compare
pengx17
force-pushed
the
xp/04-24-refactor_electron_nestjsfy
branch
from
bf829df to
68ab87f
Compare
forehalo
force-pushed
the
canary
branch
4 times, most recently
from
1c0c9fc to
8d77c48
Compare
fix AF-2509
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores
PR Dependency Tree
This tree was auto-generated by Charcoal