From a1379f9130c4d43eb8a30bfa1790684b4e5c35d8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Caleb=20=E3=83=84=20Everett?= Date: Thu, 28 Aug 2025 11:00:18 -0700 Subject: [PATCH] feat: add public close methods Add method to close transport and writer. Before there was no public way to close the transport/reader/writer created by attach for a socket, and the connection would hold the node process open. For example, this script would not exit (without an explicit process.exit(0)) ``` attach({ socket: pathToNvimSocket }) ``` This change adds .close to transport and NeovimClient which ends the writer. The other side of the connection should then close the reader which triggers detach and the cleanup code. I added asyncDispose methods for folks using modern javascript runtimes that support explicit resource management: ``` await using client = attach({ socket: pathToNvimSocket }) ``` Other wise you can call close ``` try { const client = attach({ socket: pathToNvimSocket }) } finally { await client.close() } ``` --- packages/neovim/src/api/client.ts | 12 +++++++++++ packages/neovim/src/utils/transport.test.ts | 14 ++++++++++++- packages/neovim/src/utils/transport.ts | 22 ++++++++++++++++++++- packages/neovim/src/utils/util.ts | 6 ++++++ 4 files changed, 52 insertions(+), 2 deletions(-) diff --git a/packages/neovim/src/api/client.ts b/packages/neovim/src/api/client.ts index 26b3a42d..66603886 100644 --- a/packages/neovim/src/api/client.ts +++ b/packages/neovim/src/api/client.ts @@ -6,6 +6,7 @@ import { Transport } from '../utils/transport'; import { VimValue } from '../types/VimValue'; import { Neovim } from './Neovim'; import { Buffer } from './Buffer'; +import { ASYNC_DISPOSE_SYMBOL } from '../utils/util'; const REGEX_BUF_EVENT = /nvim_buf_(.*)_event/; @@ -249,4 +250,15 @@ export class NeovimClient extends Neovim { return false; } + + async close(): Promise { + await this.transport.close(); + } + + /** + * @see close + */ + async [ASYNC_DISPOSE_SYMBOL](): Promise { + await this.close(); + } } diff --git a/packages/neovim/src/utils/transport.test.ts b/packages/neovim/src/utils/transport.test.ts index 46aa399f..2782dc4d 100644 --- a/packages/neovim/src/utils/transport.test.ts +++ b/packages/neovim/src/utils/transport.test.ts @@ -1,5 +1,5 @@ import { EventEmitter } from 'node:events'; -import { Readable, Writable } from 'node:stream'; +import { PassThrough, Readable, Writable } from 'node:stream'; import * as msgpack from '@msgpack/msgpack'; import expect from 'expect'; import { attach } from '../attach/attach'; @@ -27,4 +27,16 @@ describe('transport', () => { const msg = msgpack.encode(invalidPayload); fakeReader.push(Buffer.from(msg.buffer, msg.byteOffset, msg.byteLength)); }); + + it('closes transport and cleans up pending requests', async () => { + const socket = new PassThrough(); + + const nvim = attach({ reader: socket, writer: socket }); + + // Close the transport + const closePromise = nvim.close(); + + // Verify close promise resolves + await expect(closePromise).resolves.toBeUndefined(); + }); }); diff --git a/packages/neovim/src/utils/transport.ts b/packages/neovim/src/utils/transport.ts index 66fbe138..f2b64862 100644 --- a/packages/neovim/src/utils/transport.ts +++ b/packages/neovim/src/utils/transport.ts @@ -7,6 +7,7 @@ import { inspect } from 'node:util'; import { encode, decode, ExtensionCodec, decodeMultiStream } from '@msgpack/msgpack'; import { Metadata } from '../api/types'; +import { ASYNC_DISPOSE_SYMBOL } from './util'; export let exportsForTesting: any; // eslint-disable-line import/no-mutable-exports // .mocharc.js sets NODE_ENV=test. @@ -88,7 +89,7 @@ class Transport extends EventEmitter { this.reader = reader; this.client = client; - this.reader.on('end', () => { + this.reader.once('end', () => { this.emit('detach'); }); @@ -179,6 +180,25 @@ class Transport extends EventEmitter { this.writer.write(this.encodeToBuffer([1, 0, 'Invalid message type', null])); } } + + /** + * Close the transport. + * + * Ends the writer, the other end of the connection should close our reader which cleans up + * remaining resources. + */ + async close(): Promise { + return new Promise(resolve => { + this.writer.end(resolve); + }); + } + + /** + * @see close + */ + async [ASYNC_DISPOSE_SYMBOL](): Promise { + await this.close(); + } } export { Transport }; diff --git a/packages/neovim/src/utils/util.ts b/packages/neovim/src/utils/util.ts index cc2e428e..e3dbc754 100644 --- a/packages/neovim/src/utils/util.ts +++ b/packages/neovim/src/utils/util.ts @@ -41,3 +41,9 @@ export function partialClone( return clonedObj; } + +/** + * Polyfill for Symbol.asyncDispose if not available in the runtime. + * https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Symbol/asyncDispose + */ +export const ASYNC_DISPOSE_SYMBOL = Symbol.asyncDispose ?? Symbol.for('Symbol.asyncDispose');