Repository navigation
Isolate the websocket and jsonrpc work into its own package - #115
Conversation
d757f79 to
7fc6a4e
Compare
|
|
||
| export interface ItemRef { | ||
| root_id: string; | ||
| prim_id?: string | null; |
There was a problem hiding this comment.
Hard to verify, but I think either this or RuntimeError.prim_id and RuntimeDebug.prim_id should also be nullable?
| prim_id?: string | null; | |
| prim_id?: string; |
There was a problem hiding this comment.
This needs further investigation. I will leave it as is for the moment, as this pr is mostly about extracting the functionality.
| isConnecting(): boolean; | ||
| onConnectionChange(listener: (event: ConnectionChangeEvent) => any): Disposable; | ||
| on(method: string, handler: ((params?: any) => any | Promise<any> | void) | undefined): void; | ||
| call(method: string, params?: any): Promise<any>; |
There was a problem hiding this comment.
Nice to see this split out.
I'm happy to raise a later PR that would improve the types here if that'd be appreciated, once this lands? I know you're still working on it for separate changes so will wait and see what lands there.
Additionally @WolfGangS, I imagine you have plans to make ws an interface? MessageTransport is already there, so the socket looks like the last piece tied to Node.
There was a problem hiding this comment.
I have no specific plans at the moment, i just wanted to get the ground work done for now.
I have no immediate use for this, I just know you and some others do, and saw it as something i could hash out the first version of quickly.
7fc6a4e to
1b51068
Compare
1b51068 to
5811028
Compare
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Moderate build, packaging, dependency, and connection-lifecycle issues remain unresolved.
Get a fresh assessment by requesting another Copilot review.
Review effort: Lite
Findings: 1
Open (1)
What changed in this PR
This PR extracts WebSocket/JSON-RPC and object-content functionality into the reusable @secondlife/sl-ide-ws-client package and updates the extension to consume it.
Changes:
- Added package protocols, transports, events, clients, and object-content APIs.
- Updated extension services, tests, imports, and documentation.
- Added workspace build, lint, and packaging configuration.
Review findings:
package.json:418— Moderate (1 vote): the watch workflow does not rebuild the workspace package.packages/sl-ide-ws-client/README.md:132— Nit (1 vote): the example callssetup()twice and removes prior handlers.packages/sl-ide-ws-client/package.json:23— Moderate (2 votes): published packages omit the linked documentation.packages/sl-ide-ws-client/package.json:35— Moderate (1 vote): public declarations require@types/wsas a runtime dependency.packages/sl-ide-ws-client/src/viewereditwsclient.ts:189— Moderate (2 votes):setup()implicitly connects while callers may immediately proceed.
| File | Reviewed change |
|---|---|
src/vscode/objectexplorerwebview.ts |
Uses package client types. |
src/vscode/objectexplorerprovider.ts |
Uses package object-content APIs. |
src/vscode/objectcontentprovider.ts |
Uses package models and client. |
src/vscode/ObjectContentDecorator.ts |
Uses the packaged service. |
src/test/suite/objectcontentservice.test.ts |
Tests the packaged service. |
src/synchservice.ts |
Integrates the packaged client and protocols. |
src/shared/languageservice.ts |
Uses packaged JSON-RPC types. |
src/scriptsync.ts |
Uses packaged runtime types. |
src/extension.ts |
Uses packaged services. |
src/commandregistry.ts |
Uses packaged RPC types. |
README.md |
Updates protocol documentation links. |
packages/sl-ide-ws-client/tsconfig.json |
Adds package compiler configuration. |
packages/sl-ide-ws-client/src/websockclient.ts |
Extracts the base WebSocket client. |
packages/sl-ide-ws-client/src/viewereditwsclient.ts |
Provides the standalone viewer client. |
packages/sl-ide-ws-client/src/transport.ts |
Adds transport abstractions. |
packages/sl-ide-ws-client/src/protocol.ts |
Defines protocol interfaces. |
packages/sl-ide-ws-client/src/objectcontentservice.ts |
Extracts the portable object service. |
packages/sl-ide-ws-client/src/objectcontentinterfaces.ts |
Hosts object-content types. |
packages/sl-ide-ws-client/src/jsonrpcclient.ts |
Extracts the JSON-RPC implementation. |
packages/sl-ide-ws-client/src/index.ts |
Exports package APIs. |
packages/sl-ide-ws-client/src/events.ts |
Adds portable event primitives. |
packages/sl-ide-ws-client/README.md |
Documents package usage. |
packages/sl-ide-ws-client/package.json |
Defines package metadata and exports. |
packages/sl-ide-ws-client/doc/Message_Interfaces.md |
Moves protocol documentation. |
package.json |
Adds workspace and package build integration. |
eslint.config.mjs |
Adds package lint configuration. |
.vscodeignore |
Controls packaged workspace contents. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Rider-Linden
left a comment
Rider-Linden
left a comment
There was a problem hiding this comment.
My only reservation is moving the protocol file. (I noticed that after chiding copilot)
Not a blocker.
A first pass at isolating the websocket work into its own package, to allow it to be spun of as a library for other users.
This makes no real changes to the websocket, some parts have been broken out into their own files rather than having multiple classes in the same file.
I also moved the
Messages_Interfaces.mdinto the websocket client package.Currently the package is included via an import to avoid symlinking as that requires modifying the github action to make
vscefollow symlinks for builds, as I'd leave that decision to LindenLabThis would be a first step to full filling #113