Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions packages/neovim/src/api/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
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/;

Expand Down Expand Up @@ -249,4 +250,15 @@

return false;
}

async close(): Promise<void> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

await this.transport.close();
}

/**
* @see close
*/
async [ASYNC_DISPOSE_SYMBOL](): Promise<void> {
await this.close();
}

Check warning on line 263 in packages/neovim/src/api/client.ts

View check run for this annotation

Codecov / codecov/patch

packages/neovim/src/api/client.ts#L262-L263

Added lines #L262 - L263 were not covered by tests
}
14 changes: 13 additions & 1 deletion packages/neovim/src/utils/transport.test.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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();
});
});
22 changes: 21 additions & 1 deletion packages/neovim/src/utils/transport.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

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.
Expand Down Expand Up @@ -88,7 +89,7 @@
this.reader = reader;
this.client = client;

this.reader.on('end', () => {
this.reader.once('end', () => {
this.emit('detach');
Comment on lines -91 to 93
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice change, I wonder if this will fix some test flakiness that happens sometimes

});

Expand Down Expand Up @@ -179,6 +180,25 @@
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<void> {
return new Promise(resolve => {
this.writer.end(resolve);
});
}

/**
* @see close
*/
async [ASYNC_DISPOSE_SYMBOL](): Promise<void> {
await this.close();
}

Check warning on line 201 in packages/neovim/src/utils/transport.ts

View check run for this annotation

Codecov / codecov/patch

packages/neovim/src/utils/transport.ts#L200-L201

Added lines #L200 - L201 were not covered by tests
}

export { Transport };
6 changes: 6 additions & 0 deletions packages/neovim/src/utils/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Loading