refactor: make rslint compatible with browser#303
Conversation
✅ Deploy Preview for rslint canceled.
|
There was a problem hiding this comment.
Pull Request Overview
This PR refactors the rslint service to support browser environments by abstracting the service implementation behind a common interface. The changes enable rslint to run in both Node.js (using child processes) and browser environments (using web workers), preparing the codebase for cross-platform compatibility.
Key changes:
- Split the monolithic service into environment-specific implementations (Node.js and browser)
- Introduced a factory pattern to automatically select the appropriate service implementation
- Created shared type definitions for IPC protocol consistency across environments
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/rslint/src/worker.ts | New web worker implementation for browser environment with simulated rslint process communication |
| packages/rslint/src/types.ts | Extracted shared type definitions for IPC protocol and service interfaces |
| packages/rslint/src/service.ts | Refactored main service to use factory pattern and delegate to environment-specific implementations |
| packages/rslint/src/node.ts | Extracted Node.js-specific service implementation using child processes |
| packages/rslint/src/index.ts | Updated exports to include new service implementations and types |
| packages/rslint/src/browser.ts | New browser service implementation using web workers |
| packages/rslint/package.json | Updated package exports to support browser field and worker entry point |
| for (const [id, pending] of pendingMessages) { | ||
| self.postMessage({ | ||
| id, | ||
| kind: 'error', | ||
| data: { | ||
| message: `Worker error: ${error.message}`, | ||
| }, | ||
| }); | ||
| } |
There was a problem hiding this comment.
The pendingMessages variable is declared globally but never populated in this worker implementation. The error handling loop will never execute because the Map is always empty.
|
|
||
| // Handle the message | ||
| try { | ||
| const parsed: IpcMessage = JSON.parse(message); |
There was a problem hiding this comment.
JSON.parse() is called without validation on data received from an external process. Consider adding schema validation to ensure the parsed data conforms to the IpcMessage interface.
| /** | ||
| * Handle messages from the worker | ||
| */ | ||
| private handleWorkerMessage(message: IpcMessage): void { |
There was a problem hiding this comment.
The worker message handler doesn't validate that the incoming message conforms to the IpcMessage interface. Consider adding runtime validation for messages received from the worker.
| private handleWorkerMessage(message: IpcMessage): void { | |
| private handleWorkerMessage(message: any): void { | |
| if (!isIpcMessage(message)) { | |
| console.error('Received invalid message from worker:', message); | |
| return; | |
| } |
make service compatible with web worker and prepare for browser support