refactor: DNM WIP - context isolation structural example - #8456
Conversation
| ), | ||
| }; | ||
|
|
||
| globalThis.postMessage(bootMessage, '*', [channel.port2]); |
|
How to follow this:
The path from here sort of follows the very minimal non-example in the code:
|
97e9c05 to
98666fa
Compare
There was a problem hiding this comment.
🟡 Changes recommended
The new mongodb-data-service connect() path is not safely gated to Electron and can break/hang Node-based tests/consumers, and the new IPC bridge needs basic validation/robustness improvements.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR prototypes a split DataService architecture (renderer façade + Electron utility process backend) as a stepping stone toward contextIsolation: true / nodeIntegration: false, while aiming to keep the current Compass behavior largely intact.
Changes:
- Introduces
DataServiceRenderer/DataServiceUtilityand a MessagePort-based bootstrapping path between renderer → preload → main → utility process. - Wires up a preload script and launches a dedicated utility process for the data service from the main process.
- Extends build/lint tooling to support
.mtsand emits additional bundles (preload.js,data-service.mjs).
File summaries
| File | Description |
|---|---|
| packages/data-service/src/index.ts | Exposes the new utility-side DataService wrapper from the package entrypoint. |
| packages/data-service/src/data-service-utility.ts | Adds a utility-process DataService subclass that listens on a MessagePortMain. |
| packages/data-service/src/data-service-renderer.ts | Adds a renderer-side DataService subclass that communicates via MessageChannel + postMessage. |
| packages/data-service/src/connect.ts | Switches the library connect() helper to use the new renderer implementation. |
| packages/compass/webpack.config.js | Builds additional entrypoints for the utility process and preload script. |
| packages/compass/src/utilities/data-service/index.mts | Adds the Electron utility process entrypoint that hosts DataServiceUtility. |
| packages/compass/src/preload/index.ts | Adds a preload entrypoint that installs the port bridge. |
| packages/compass/src/preload/data-service-port-bridge.ts | Forwards the renderer-posted MessagePort to main via ipcRenderer.postMessage. |
| packages/compass/src/main/window-manager.ts | Configures the BrowserWindow preload script. |
| packages/compass/src/main/application.ts | Launches the utility process and relays the port to it from ipcMain. |
| configs/webpack-config-compass/src/loaders.ts | Updates webpack loader patterns to recognize .mts. |
| configs/eslint-config-compass/index.js | Updates TS overrides so .mts/.cts are linted as TypeScript. |
Review details
Suppressed comments (1)
packages/data-service/src/connect.ts:26
DataServiceRenderershould not be instantiated unconditionally: in non-Electron environments there is no preload/utility bridge, so the constructor/connect path can throw or hang. Choose the implementation at runtime (e.g., based on the Electron user agent) and fall back toDataServiceImpl.
const dataService = new DataServiceRenderer(
connectionOptions,
logger,
proxyOptions
);
- Files reviewed: 12/12 changed files
- Comments generated: 4
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| if ( | ||
| event.source === window && | ||
| event.data?.type === 'compass:data-service:port' | ||
| ) { | ||
| ipcRenderer.postMessage('compass:data-service:port', event.data, [ | ||
| ...event.ports, | ||
| ]); | ||
| } |
| setInterval(() => { | ||
| // eslint-disable-next-line no-console | ||
| console.log(new Date(), 'hello from utility'); | ||
| }, 5000); |
| import type { DataService } from './data-service'; | ||
| import type { DataServiceImplLogger } from './logger'; | ||
| import { DataServiceImpl } from './data-service'; | ||
| import { DataServiceRenderer } from './data-service-renderer'; |
| private async send(message: any) { | ||
| const reply = once(this.portToUtility, 'message'); | ||
| this.portToUtility.postMessage(message); | ||
| const [response] = await reply; | ||
| return response; | ||
| } |
| outputFilename: '[name].js', | ||
| }); | ||
| // See the comment above `dataServiceUtilityBaseConfig`: this is a second | ||
| // `electron-main`-targeted config, so it would otherwise steal the |
There was a problem hiding this comment.
But the target is electron-preload? 🤔 We can probably just adjust the plugin to give it more hints about what to run when we're doing a proper implementation
| * ▝▀ ▀ ▀▘▘▀▘▀ ▗▄▘ * | ||
| ********************/ | ||
|
|
||
| // There should be no changes. |
Description
This demonstrates how we begin linking a renderer data service to a utility data service to
get Compass closer to contextIsolation:true. I think its interesting to see the simple example
working in a way that doesn't impact Compass's current implementation so that its apparent
how this could be done progressively.
Motivation and Context
Context isolation and nodeIntegration:false will make the renderer purely a web-only
application and draw an uncrossable line between it and system resources.
Open Questions
Still needs clarity:
Stack created with GitHub Stacks CLI • Give Feedback 💬