fix(xml-tools): resolve LSP client startup crash (IPCMessageReader) - #636
Conversation
…client Signed-off-by: badrislamovrolan <rolan.badrislamov@sap.com>
Build ReportPlease note:
|
…client Signed-off-by: badrislamovrolan <rolan.badrislamov@sap.com>
jacob-kreyenbuehl
left a comment
There was a problem hiding this comment.
Reviewed the change end to end. The fix is correct and the version choice is right. Two items to fix, one to confirm.
Should fix: the reason in the description is not the real one.
IPCMessageReader is not part of vscode-languageserver-protocol. It lives in vscode-jsonrpc, and it is still there in 6.0.0, 8.2.0 and 9.0.2. What changed in 3.18.x is the entry point. 3.16.0 and 3.17.x have "main": "./lib/node/main.js", so a bare import gets the node entry with the IPC classes. 3.18.3 has an exports map where "." points to ./lib/common/api.js, the browser entry, and the node entry moved behind ./node. The client uses a bare import (vscode-languageclient/lib/main.js:338), so it got undefined.
Same version boundary, but the real rule is about the export map. Please reword, so the next version bump does not repeat this.
Should fix: add a reason for the pin. See the inline comment.
Needs verify: how does this fix reach users? xml-toolkit is private and has no tag or Release yet. The VSIX is built from main inside the release run, and uploaded only for released packages. main already has xml-toolkit 1.3.1 from the #634 changeset, and that release is still pending. So the fix rides that release if this merges first. If the backlog ships first, we need a changeset. Which case is it?
Nit: the description says this replaces the pnpm-workspace.yaml override, but main has no such override and this PR does not touch that file.
Note, not a defect: the root cause is vscode-languageclient 6.1.3, which reads node classes through a bare import. The pin is a good fix for the crash. Upgrading client and server is the forward fix.
Checked: only xml-tools consumes these packages. The client wants ^3.15.3 and the server pins 3.16.0 exactly, so one copy serves both. The lockfile resolves only 3.16.0, and pnpm install --lockfile-only shows no drift. webpack bundles the client and the protocol. CI is green. I did not repeat the VSIX test, that needs a dev space.
… pin Signed-off-by: badrislamovrolan <rolan.badrislamov@sap.com>
What
Pin
vscode-languageserver-protocolto3.16.0as a direct dependency ofxml-toolkit.Why
xml-toolkitusesvscode-languageclient@6.1.3, which bare-importsvscode-languageserver-protocoland expects the Node entry (it re-exportsIPCMessageReader/IPCMessageWriterfromvscode-jsonrpc).The client's range
^3.15.3allowed 3.18.x, so pnpm resolved 3.18.3 and webpack bundled it. The break is the entry point, not a removed symbol:3.16.0/3.17.x:"main": "./lib/node/main.js"→ bare import gets the Node entry.3.18.x:"main"replaced by an"exports"map whose"."is the common entry; the Node entry moved behind./node.Under 3.18.x the bare import returned the common entry,
IPCMessageReaderwasundefined, and the client crashed on activation:Fix
Add
vscode-languageserver-protocol: 3.16.0toxml-toolkit'sdependencies, with a//comment on why the pin must stay.3.16.0 satisfies the client's
^3.15.3, so pnpm dedupes to one copy and webpack bundles it. It exposes the Node entry via"main"and matches the server (vscode-languageserver@7.0.0). The scoped dependency limits the pin to the one package that needs it. Keep it at 3.16.0 until the client is upgraded to import the Node entry explicitly.Testing
3.16.0(was 3.18.3),IPCMessageReaderpresent..xml: no startup error; malformed XML shows squiggles + Problems entries.