From 93d53bfeb1e44841c3f50f60f6ef053a23477019 Mon Sep 17 00:00:00 2001 From: Milly Date: Tue, 30 Apr 2024 03:31:47 +0900 Subject: [PATCH 1/5] :+1: Add plugin unload features Plugin API: - `Entrypoint` can now return `AsyncDisposable`. - It is called by `denops#plugin#unload()`. - It is called when the server closing. - When `denops#server#close()` is called. - When `denops#server#stop()` is called. - When the channel is closed. - When the server process trapped `SIGINT`. Vim API: - Add new `denops-function`: - `denops#plugin#unload({plugin})` - Add new events: - `DenopsPluginUnloadPre:{plugin}` - `DenopsPluginUnloadPost:{plugin}` - `DenopsPluginUnloadFail:{plugin}` --- autoload/denops/_internal/plugin.vim | 35 ++ autoload/denops/plugin.vim | 4 + deno.jsonc | 6 + denops/@denops-private/host.ts | 4 + denops/@denops-private/host/nvim_test.ts | 1 + denops/@denops-private/host/vim_test.ts | 1 + denops/@denops-private/host_test.ts | 1 + denops/@denops-private/plugin.ts | 35 ++ denops/@denops-private/service.ts | 95 ++++-- denops/@denops-private/service_test.ts | 316 +++++++++++++++++- doc/denops.txt | 53 ++- plugin/denops.vim | 3 + plugin/denops/debug.vim | 4 +- tests/denops/runtime/functions/plugin_test.ts | 206 +++++++++++- .../testdata/dummy_invalid_dispose_plugin.ts | 11 + .../testdata/dummy_valid_dispose_plugin.ts | 11 + 16 files changed, 748 insertions(+), 38 deletions(-) create mode 100644 denops/@denops-private/plugin.ts create mode 100644 tests/denops/testdata/dummy_invalid_dispose_plugin.ts create mode 100644 tests/denops/testdata/dummy_valid_dispose_plugin.ts diff --git a/autoload/denops/_internal/plugin.vim b/autoload/denops/_internal/plugin.vim index 73d96ba5..8bde492b 100644 --- a/autoload/denops/_internal/plugin.vim +++ b/autoload/denops/_internal/plugin.vim @@ -1,6 +1,7 @@ const s:STATE_RESERVED = 'reserved' const s:STATE_LOADING = 'loading' const s:STATE_LOADED = 'loaded' +const s:STATE_UNLOADING = 'unloading' const s:STATE_FAILED = 'failed' let s:plugins = {} @@ -27,6 +28,14 @@ function! denops#_internal#plugin#load(name, script) abort call denops#_internal#server#chan#notify('invoke', ['load', l:args]) endfunction +function! denops#_internal#plugin#unload(name) abort + let l:args = [a:name] + let l:plugin = denops#_internal#plugin#get(a:name) + let l:plugin.state = s:STATE_UNLOADING + call denops#_internal#echo#debug(printf('unload plugin: %s', l:args)) + call denops#_internal#server#chan#notify('invoke', ['unload', l:args]) +endfunction + function! denops#_internal#plugin#reload(name) abort const l:args = [a:name] let l:plugin = denops#_internal#plugin#get(a:name) @@ -60,10 +69,36 @@ function! s:DenopsSystemPluginFail() abort execute printf('doautocmd User DenopsPluginFail:%s', l:name) endfunction +function! s:DenopsSystemPluginUnloadPre() abort + const l:name = matchstr(expand(''), 'DenopsSystemPluginUnloadPre:\zs.*') + let l:plugin = denops#_internal#plugin#get(l:name) + let l:plugin.state = s:STATE_UNLOADING + execute printf('doautocmd User DenopsPluginUnloadPre:%s', l:name) +endfunction + +function! s:DenopsSystemPluginUnloadPost() abort + const l:name = matchstr(expand(''), 'DenopsSystemPluginUnloadPost:\zs.*') + let l:plugin = denops#_internal#plugin#get(l:name) + let l:plugin.state = s:STATE_RESERVED + let l:plugin.callbacks = [] + execute printf('doautocmd User DenopsPluginUnloadPost:%s', l:name) +endfunction + +function! s:DenopsSystemPluginUnloadFail() abort + const l:name = matchstr(expand(''), 'DenopsSystemPluginUnloadFail:\zs.*') + let l:plugin = denops#_internal#plugin#get(l:name) + let l:plugin.state = s:STATE_FAILED + let l:plugin.callbacks = [] + execute printf('doautocmd User DenopsPluginUnloadFail:%s', l:name) +endfunction + augroup denops_autoload_plugin_internal autocmd! autocmd User DenopsSystemPluginPre:* ++nested call s:DenopsSystemPluginPre() autocmd User DenopsSystemPluginPost:* ++nested call s:DenopsSystemPluginPost() autocmd User DenopsSystemPluginFail:* ++nested call s:DenopsSystemPluginFail() + autocmd User DenopsSystemPluginUnloadPre:* ++nested call s:DenopsSystemPluginUnloadPre() + autocmd User DenopsSystemPluginUnloadPost:* ++nested call s:DenopsSystemPluginUnloadPost() + autocmd User DenopsSystemPluginUnloadFail:* ++nested call s:DenopsSystemPluginUnloadFail() autocmd User DenopsClosed let s:plugins = {} augroup END diff --git a/autoload/denops/plugin.vim b/autoload/denops/plugin.vim index 27b1998e..84ca02c9 100644 --- a/autoload/denops/plugin.vim +++ b/autoload/denops/plugin.vim @@ -53,6 +53,10 @@ function! denops#plugin#load(name, script) abort call denops#_internal#plugin#load(a:name, a:script) endfunction +function! denops#plugin#unload(name) abort + call denops#_internal#plugin#unload(a:name) +endfunction + function! denops#plugin#reload(name) abort call denops#_internal#plugin#reload(a:name) endfunction diff --git a/deno.jsonc b/deno.jsonc index a084a759..1e196b23 100644 --- a/deno.jsonc +++ b/deno.jsonc @@ -7,6 +7,12 @@ "update": "deno run --allow-env --allow-read --allow-write=. --allow-run=git,deno --allow-net=jsr.io,registry.npmjs.org jsr:@molt/cli **/*.ts", "update:commit": "deno task -q update --commit --pre-commit=fmt,lint" }, + "test": { + "exclude": [ + // TODO: #349 Update `Entrypoint` in denops-core, and remove this entry. + "denops/@denops-private/plugin.ts" + ] + }, "exclude": [ ".coverage/" ], diff --git a/denops/@denops-private/host.ts b/denops/@denops-private/host.ts index d0f57b49..93887158 100644 --- a/denops/@denops-private/host.ts +++ b/denops/@denops-private/host.ts @@ -49,6 +49,7 @@ export type CallbackId = string; export type Service = { bind(host: Host): void; load(name: string, script: string): Promise; + unload(name: string): Promise; reload(name: string): Promise; interrupt(reason?: unknown): void; dispatch(name: string, fn: string, args: unknown[]): Promise; @@ -72,6 +73,8 @@ export function invoke( switch (name) { case "load": return service.load(...ensure(args, serviceMethodArgs.load)); + case "unload": + return service.unload(...ensure(args, serviceMethodArgs.unload)); case "reload": return service.reload(...ensure(args, serviceMethodArgs.reload)); case "interrupt": @@ -92,6 +95,7 @@ export function invoke( const serviceMethodArgs = { load: is.ParametersOf([is.String, is.String] as const), + unload: is.ParametersOf([is.String] as const), reload: is.ParametersOf([is.String] as const), interrupt: is.ParametersOf([is.OptionalOf(is.Unknown)] as const), dispatch: is.ParametersOf([is.String, is.String, is.Array] as const), diff --git a/denops/@denops-private/host/nvim_test.ts b/denops/@denops-private/host/nvim_test.ts index 6ee52f1a..859e5de7 100644 --- a/denops/@denops-private/host/nvim_test.ts +++ b/denops/@denops-private/host/nvim_test.ts @@ -23,6 +23,7 @@ Deno.test("Neovim", async (t) => { const service: Service = { bind: () => unimplemented(), load: () => unimplemented(), + unload: () => unimplemented(), reload: () => unimplemented(), interrupt: () => unimplemented(), dispatch: () => unimplemented(), diff --git a/denops/@denops-private/host/vim_test.ts b/denops/@denops-private/host/vim_test.ts index 74b2eb9f..3c31176f 100644 --- a/denops/@denops-private/host/vim_test.ts +++ b/denops/@denops-private/host/vim_test.ts @@ -18,6 +18,7 @@ Deno.test("Vim", async (t) => { const service: Service = { bind: () => unimplemented(), load: () => unimplemented(), + unload: () => unimplemented(), reload: () => unimplemented(), interrupt: () => unimplemented(), dispatch: () => unimplemented(), diff --git a/denops/@denops-private/host_test.ts b/denops/@denops-private/host_test.ts index b4d87f76..a7989435 100644 --- a/denops/@denops-private/host_test.ts +++ b/denops/@denops-private/host_test.ts @@ -11,6 +11,7 @@ import { invoke, type Service } from "./host.ts"; Deno.test("invoke", async (t) => { const service: Omit = { load: () => unimplemented(), + unload: () => unimplemented(), reload: () => unimplemented(), interrupt: () => unimplemented(), dispatch: () => unimplemented(), diff --git a/denops/@denops-private/plugin.ts b/denops/@denops-private/plugin.ts new file mode 100644 index 00000000..00c9441b --- /dev/null +++ b/denops/@denops-private/plugin.ts @@ -0,0 +1,35 @@ +// TODO: #349 Update `Entrypoint` in denops-core, remove this module from `$.test.exclude` in `deno.jsonc`, and remove this module. +import type { Denops } from "jsr:@denops/core@6.1.0"; + +/** + * Denops's entrypoint definition. + * + * Use this type to ensure the `main` function is properly implemented like: + * + * ```ts + * import type { Entrypoint } from "jsr:@denops/core"; + * + * export const main: Entrypoint = (denops) => { + * // ... + * } + * ``` + * + * If an `AsyncDisposable` object is returned, resources can be disposed of + * asynchronously when the plugin is unloaded, like: + * + * ```ts + * import type { Entrypoint } from "jsr:@denops/core"; + * + * export const main: Entrypoint = (denops) => { + * // ... + * return { + * [Symbol.asyncDispose]: () => { + * // Dispose resources... + * } + * } + * } + * ``` + */ +export type Entrypoint = ( + denops: Denops, +) => void | AsyncDisposable | Promise; diff --git a/denops/@denops-private/service.ts b/denops/@denops-private/service.ts index fc380917..4552a5cd 100644 --- a/denops/@denops-private/service.ts +++ b/denops/@denops-private/service.ts @@ -1,3 +1,6 @@ +// TODO: #349 Import `Entrypoint` from denops-core. +// import type { Entrypoint } from "jsr:@denops/core@7.0.0"; +import type { Entrypoint } from "./plugin.ts"; import type { Denops, Meta } from "jsr:@denops/core@6.0.6"; import { toFileUrl } from "jsr:@std/path@0.225.0/to-file-url"; import { toErrorObject } from "jsr:@lambdalisue/errorutil@1.0.0"; @@ -38,11 +41,7 @@ export class Service implements HostService, AsyncDisposable { this.#host = host; } - async load( - name: string, - script: string, - suffix = "", - ): Promise { + async load(name: string, script: string): Promise { if (this.#closed) { throw new Error("Service closed"); } @@ -58,26 +57,36 @@ export class Service implements HostService, AsyncDisposable { const denops = new DenopsImpl(name, this.#meta, this.#host, this); const plugin = new Plugin(denops, name, script); this.#plugins.set(name, plugin); - await plugin.load(suffix); - this.#getWaiter(name).resolve(); + try { + await plugin.waitLoaded(); + this.#getWaiter(name).resolve(); + } catch { + this.#plugins.delete(name); + } } - reload( - name: string, - ): Promise { + async #unload(name: string): Promise { const plugin = this.#plugins.get(name); if (!plugin) { if (this.#meta.mode === "debug") { console.log(`A denops plugin '${name}' is not loaded yet. Skip`); } - return Promise.resolve(); + return; } this.#plugins.delete(name); - this.#waiters.delete(name); - // Import module with fragment so that reload works properly - // https://github.com/vim-denops/denops.vim/issues/227 - const suffix = `#${performance.now()}`; - return this.load(name, plugin.script, suffix); + await plugin.unload(); + return plugin; + } + + async unload(name: string): Promise { + await this.#unload(name); + } + + async reload(name: string): Promise { + const plugin = await this.#unload(name); + if (plugin) { + await this.load(name, plugin.script); + } } waitLoaded(name: string): Promise { @@ -137,7 +146,7 @@ export class Service implements HostService, AsyncDisposable { } } - close(): Promise { + async close(): Promise { if (!this.#closed) { this.#closed = true; const error = new Error("Service closed"); @@ -145,6 +154,9 @@ export class Service implements HostService, AsyncDisposable { reject(error); } this.#waiters.clear(); + await Promise.all( + [...this.#plugins.values()].map((plugin) => plugin.unload()), + ); this.#plugins.clear(); this.#host = undefined; this.#closedWaiter.resolve(); @@ -161,8 +173,14 @@ export class Service implements HostService, AsyncDisposable { } } +type PluginModule = { + main: Entrypoint; +}; + class Plugin { #denops: Denops; + #loadedWaiter: Promise; + #disposable: Partial = {}; readonly name: string; readonly script: string; @@ -171,13 +189,19 @@ class Plugin { this.#denops = denops; this.name = name; this.script = resolveScriptUrl(script); + this.#loadedWaiter = this.#load(); } - async load(suffix = ""): Promise { + waitLoaded(): Promise { + return this.#loadedWaiter; + } + + async #load(): Promise { + const suffix = createScriptSuffix(this.script); try { await emit(this.#denops, `DenopsSystemPluginPre:${this.name}`); - const mod = await import(`${this.script}${suffix}`); - await mod.main(this.#denops); + const mod: PluginModule = await import(`${this.script}${suffix}`); + this.#disposable = await mod.main(this.#denops) ?? {}; await emit(this.#denops, `DenopsSystemPluginPost:${this.name}`); } catch (e) { // Show a warning message when Deno module cache issue is detected @@ -195,6 +219,27 @@ class Plugin { } console.error(`Failed to load plugin '${this.name}': ${e}`); await emit(this.#denops, `DenopsSystemPluginFail:${this.name}`); + throw e; + } + } + + async unload(): Promise { + try { + // Wait for the load to complete to make the events atomically. + await this.#loadedWaiter; + } catch { + // Load failed, do nothing + return; + } + try { + await emit(this.#denops, `DenopsSystemPluginUnloadPre:${this.name}`); + await this.#disposable[Symbol.asyncDispose]?.(); + await emit(this.#denops, `DenopsSystemPluginUnloadPost:${this.name}`); + } catch (e) { + console.error(`Failed to unload plugin '${this.name}': ${e}`); + await emit(this.#denops, `DenopsSystemPluginUnloadFail:${this.name}`); + } finally { + this.#disposable = {}; } } @@ -210,6 +255,16 @@ class Plugin { } } +const loadedScripts = new Set(); + +function createScriptSuffix(script: string): string { + // Import module with fragment so that reload works properly + // https://github.com/vim-denops/denops.vim/issues/227 + const suffix = loadedScripts.has(script) ? `#${performance.now()}` : ""; + loadedScripts.add(script); + return suffix; +} + async function emit(denops: Denops, name: string): Promise { try { await denops.cmd(`doautocmd User ${name}`); diff --git a/denops/@denops-private/service_test.ts b/denops/@denops-private/service_test.ts index 0e2b4c24..5a778463 100644 --- a/denops/@denops-private/service_test.ts +++ b/denops/@denops-private/service_test.ts @@ -1,5 +1,6 @@ import { assert, + assertArrayIncludes, assertEquals, assertFalse, assertInstanceOf, @@ -13,6 +14,7 @@ import { assertSpyCall, assertSpyCalls, resolvesNext, + spy, stub, } from "jsr:@std/testing@0.224.0/mock"; import type { Meta } from "jsr:@denops/core@6.0.6"; @@ -25,6 +27,8 @@ const NOOP = () => {}; const scriptValid = resolve("dummy_valid_plugin.ts"); const scriptInvalid = resolve("dummy_invalid_plugin.ts"); +const scriptValidDispose = resolve("dummy_valid_dispose_plugin.ts"); +const scriptInvalidDispose = resolve("dummy_invalid_dispose_plugin.ts"); const scriptInvalidConstraint = resolve("dummy_invalid_constraint_plugin.ts"); const scriptInvalidConstraint2 = resolve("dummy_invalid_constraint_plugin2.ts"); @@ -301,6 +305,225 @@ Deno.test("Service", async (t) => { assertSpyCalls(host_call, 0); }); }); + + await t.step("if the plugin is already unloaded", async (t) => { + const service = new Service(meta); + service.bind(host); + { + using _host_call = stub(host, "call"); + await service.load("dummy", scriptValid); + await service.unload("dummy"); + } + using host_call = stub(host, "call"); + + await t.step("resolves", async () => { + await service.load("dummy", scriptValid); + }); + + await t.step("emits DenopsSystemPluginPre", () => { + assertSpyCall(host_call, 0, { + args: [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginPre:dummy", + {}, + ], + }); + }); + + await t.step("calls the plugin entrypoint", () => { + assertSpyCall(host_call, 1, { + args: [ + "denops#api#cmd", + "echo 'Hello, Denops!'", + {}, + ], + }); + }); + + await t.step("emits DenopsSystemPluginPost", () => { + assertSpyCall(host_call, 2, { + args: [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginPost:dummy", + {}, + ], + }); + }); + }); + }); + + await t.step(".unload()", async (t) => { + await t.step("if the plugin returns void", async (t) => { + const service = new Service(meta); + service.bind(host); + { + using _host_call = stub(host, "call"); + await service.load("dummy", scriptValid); + } + using host_call = stub(host, "call"); + + await t.step("resolves", async () => { + await service.unload("dummy"); + }); + + await t.step("emits DenopsSystemPluginUnloadPre", () => { + assertSpyCall(host_call, 0, { + args: [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginUnloadPre:dummy", + {}, + ], + }); + }); + + await t.step("emits DenopsSystemPluginUnloadPost", () => { + assertSpyCall(host_call, 1, { + args: [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginUnloadPost:dummy", + {}, + ], + }); + }); + }); + + await t.step("if the plugin returns AsyncDisposable", async (t) => { + const service = new Service(meta); + service.bind(host); + { + using _host_call = stub(host, "call"); + await service.load("dummy", scriptValidDispose); + } + using host_call = stub(host, "call"); + + await t.step("resolves", async () => { + await service.unload("dummy"); + }); + + await t.step("emits DenopsSystemPluginUnloadPre", () => { + assertSpyCall(host_call, 0, { + args: [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginUnloadPre:dummy", + {}, + ], + }); + }); + + await t.step("calls the plugin dispose method", () => { + assertSpyCall(host_call, 1, { + args: [ + "denops#api#cmd", + "echo 'Goodbye, Denops!'", + {}, + ], + }); + }); + + await t.step("emits DenopsSystemPluginUnloadPost", () => { + assertSpyCall(host_call, 2, { + args: [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginUnloadPost:dummy", + {}, + ], + }); + }); + }); + + await t.step("if the plugin dispose method throws", async (t) => { + const service = new Service(meta); + service.bind(host); + { + using _host_call = stub(host, "call"); + await service.load("dummy", scriptInvalidDispose); + } + using console_error = stub(console, "error"); + using host_call = stub(host, "call"); + + await t.step("resolves", async () => { + await service.unload("dummy"); + }); + + await t.step("outputs an error message", () => { + assertSpyCall(console_error, 0, { + args: [ + "Failed to unload plugin 'dummy': Error: This is dummy error in async dispose", + ], + }); + }); + + await t.step("emits DenopsSystemPluginUnloadPre", () => { + assertSpyCall(host_call, 0, { + args: [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginUnloadPre:dummy", + {}, + ], + }); + }); + + await t.step("emits DenopsSystemPluginUnloadFail", () => { + assertSpyCall(host_call, 1, { + args: [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginUnloadFail:dummy", + {}, + ], + }); + }); + }); + + await t.step("if the plugin is not yet loaded", async (t) => { + const service = new Service(meta); + service.bind(host); + using console_log = stub(console, "log"); + using host_call = stub(host, "call"); + + await t.step("resolves", async () => { + await service.unload("dummy"); + }); + + await t.step("outputs a log message", () => { + assertSpyCall(console_log, 0, { + args: [ + "A denops plugin 'dummy' is not loaded yet. Skip", + ], + }); + }); + + await t.step("does not calls the host", () => { + assertSpyCalls(host_call, 0); + }); + }); + + await t.step("if the plugin is already unloaded", async (t) => { + const service = new Service(meta); + service.bind(host); + { + using _host_call = stub(host, "call"); + await service.load("dummy", scriptValid); + await service.unload("dummy"); + } + using console_log = stub(console, "log"); + using host_call = stub(host, "call"); + + await t.step("resolves", async () => { + await service.unload("dummy"); + }); + + await t.step("outputs a log message", () => { + assertSpyCall(console_log, 0, { + args: [ + "A denops plugin 'dummy' is not loaded yet. Skip", + ], + }); + }); + + await t.step("does not calls the host", () => { + assertSpyCalls(host_call, 0); + }); + }); }); await t.step(".reload()", async (t) => { @@ -317,8 +540,28 @@ Deno.test("Service", async (t) => { await service.reload("dummy"); }); - await t.step("emits DenopsSystemPluginPre", () => { + await t.step("emits DenopsSystemPluginUnloadPre", () => { assertSpyCall(host_call, 0, { + args: [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginUnloadPre:dummy", + {}, + ], + }); + }); + + await t.step("emits DenopsSystemPluginUnloadPost", () => { + assertSpyCall(host_call, 1, { + args: [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginUnloadPost:dummy", + {}, + ], + }); + }); + + await t.step("emits DenopsSystemPluginPre", () => { + assertSpyCall(host_call, 2, { args: [ "denops#api#cmd", "doautocmd User DenopsSystemPluginPre:dummy", @@ -328,7 +571,7 @@ Deno.test("Service", async (t) => { }); await t.step("calls the plugin entrypoint", () => { - assertSpyCall(host_call, 1, { + assertSpyCall(host_call, 3, { args: [ "denops#api#cmd", "echo 'Hello, Denops!'", @@ -338,7 +581,7 @@ Deno.test("Service", async (t) => { }); await t.step("emits DenopsSystemPluginPost", () => { - assertSpyCall(host_call, 2, { + assertSpyCall(host_call, 4, { args: [ "denops#api#cmd", "doautocmd User DenopsSystemPluginPost:dummy", @@ -370,6 +613,34 @@ Deno.test("Service", async (t) => { assertSpyCalls(host_call, 0); }); }); + + await t.step("if the plugin is already unloaded", async (t) => { + const service = new Service(meta); + service.bind(host); + { + using _host_call = stub(host, "call"); + await service.load("dummy", scriptValid); + await service.unload("dummy"); + } + using console_log = stub(console, "log"); + using host_call = stub(host, "call"); + + await t.step("resolves", async () => { + await service.reload("dummy"); + }); + + await t.step("outputs a log message", () => { + assertSpyCall(console_log, 0, { + args: [ + "A denops plugin 'dummy' is not loaded yet. Skip", + ], + }); + }); + + await t.step("does not calls the host", () => { + assertSpyCalls(host_call, 0); + }); + }); }); await t.step(".waitLoaded()", async (t) => { @@ -810,12 +1081,47 @@ Deno.test("Service", async (t) => { await t.step(".close()", async (t) => { await t.step("if the service is not yet closed", async (t) => { const service = new Service(meta); - using _host_call = stub(host, "call"); service.bind(host); + { + using _host_call = stub(host, "call"); + await service.load("dummy", scriptValid); + await service.load("dummyDispose", scriptValidDispose); + } + using host_call = stub(host, "call"); await t.step("resolves", async () => { await service.close(); }); + + await t.step("unloads loaded plugins", () => { + assertArrayIncludes(host_call.calls.map((c) => c.args), [ + [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginUnloadPre:dummy", + {}, + ], + [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginUnloadPre:dummyDispose", + {}, + ], + [ + "denops#api#cmd", + "echo 'Goodbye, Denops!'", + {}, + ], + [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginUnloadPost:dummy", + {}, + ], + [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginUnloadPost:dummyDispose", + {}, + ], + ]); + }); }); await t.step("if the service is already closed", async (t) => { @@ -868,7 +1174,7 @@ Deno.test("Service", async (t) => { const service = new Service(meta); using _host_call = stub(host, "call"); service.bind(host); - using service_close = stub(service, "close"); + using service_close = spy(service, "close"); await t.step("resolves", async () => { await service[Symbol.asyncDispose](); diff --git a/doc/denops.txt b/doc/denops.txt index 877a8360..b661646a 100644 --- a/doc/denops.txt +++ b/doc/denops.txt @@ -409,14 +409,37 @@ denops#plugin#discover() denops#plugin#load({name}, {script}) Loads a denops plugin. Use this function to load denops plugins that are not discovered by |denops#plugin#discover()|. - It invokes |User| |DenopsPluginPre|:{plugin} just before denops - executes a "main" function of the plugin and |User| - |DenopsPluginPost|:{plugin} just after denops executes a "main" - function of the plugin. + + Loading a plugin involves the following event steps: + + - |User| |DenopsPluginPre|:{plugin} is fired. + - The plugin is loaded and the "main" function is executed. + - If it succeeds, |User| |DenopsPluginPost|:{plugin} is fired. + - If it fails, |User| |DenopsPluginFail|:{plugin} is fired. + + *denops#plugin#unload()* +denops#plugin#unload({name}) + Unloads a denops plugin. It does nothing when the plugin is not loaded + yet. + + Unloading a plugin involves the following event steps: + + - |User| |DenopsPluginUnloadPre|:{plugin} is fired. + - The plugin's dispose callback is executed, if it exists. + - If it succeeds, |User| |DenopsPluginUnloadPost|:{plugin} is fired. + - If it fails, |User| |DenopsPluginUnloadFail|:{plugin} is fired. + + The above events may not be fired if the connection is forcibly + closed or the denops server is forcibly terminated due to timeout + or other reasons. Plugins should also use the |DenopsClosed| event. *denops#plugin#reload()* -denops#plugin#reload({name}) - Reloads a denops plugin. +denops#plugin#reload({plugin}[, {options}]) + Reloads a denops plugin. It does nothing when the plugin is not loaded + yet. + + It invokes |User| autocommand events. See |denops#plugin#load()| and + |denops#plugin#unload()| for details. *denops#plugin#check_type()* denops#plugin#check_type([{name}]) @@ -483,6 +506,24 @@ DenopsPluginPre:{plugin} *DenopsPluginPre* DenopsPluginPost:{plugin} *DenopsPluginPost* Fired after the "main" function of each plugin is called. {plugin} is the name of the target plugin. + Note that if the "main" function throws an error, it will not be fired. + +DenopsPluginFail:{plugin} *DenopsPluginFail* + Fired when the "main" function of each plugin throws an error. + {plugin} is the name of the target plugin. + +DenopsPluginUnloadPre:{plugin} *DenopsPluginUnloadPre* + Fired before each plugin is unloaded. + {plugin} is the name of the target plugin. + +DenopsPluginUnloadPost:{plugin} *DenopsPluginUnloadPost* + Fired after each plugin is unloaded. + {plugin} is the name of the target plugin. + Note that if an error is thrown when unloading, it will not be fired. + +DenopsPluginUnloadFail:{plugin} *DenopsPluginUnloadFail* + Fired if an error is thrown when unloading each plugin. + {plugin} is the name of the target plugin. DenopsProcessStarted *DenopsProcessStarted* Fires when the Denops local server process is started. diff --git a/plugin/denops.vim b/plugin/denops.vim index 66d10a1b..96e40ed0 100644 --- a/plugin/denops.vim +++ b/plugin/denops.vim @@ -22,6 +22,9 @@ augroup denops_plugin_internal autocmd User DenopsPluginPre:* : autocmd User DenopsPluginPost:* : autocmd User DenopsPluginFail:* : + autocmd User DenopsPluginUnloadPre:* : + autocmd User DenopsPluginUnloadPost:* : + autocmd User DenopsPluginUnloadFail:* : autocmd User DenopsReady call denops#plugin#discover() augroup END diff --git a/plugin/denops/debug.vim b/plugin/denops/debug.vim index 6b09bb57..aa306b57 100644 --- a/plugin/denops/debug.vim +++ b/plugin/denops/debug.vim @@ -9,6 +9,8 @@ augroup denops_debug_plugin_internal \ call denops#_internal#echo#debug(expand(':t')) autocmd User DenopsStarted,DenopsListen:*,DenopsStopped:* \ call denops#_internal#echo#debug(expand(':t')) - autocmd User DenopsPluginPre:*,DenopsPluginPost:* + autocmd User DenopsPluginPre:*,DenopsPluginPost:*,DenopsPluginFail:* + \ call denops#_internal#echo#debug(expand(':t')) + autocmd User DenopsPluginUnloadPre:*,DenopsPluginUnloadPost:*,DenopsPluginUnloadFail:* \ call denops#_internal#echo#debug(expand(':t')) augroup END diff --git a/tests/denops/runtime/functions/plugin_test.ts b/tests/denops/runtime/functions/plugin_test.ts index b680fb36..76514549 100644 --- a/tests/denops/runtime/functions/plugin_test.ts +++ b/tests/denops/runtime/functions/plugin_test.ts @@ -14,6 +14,8 @@ const MESSAGE_DELAY = 200; const scriptValid = resolve("dummy_valid_plugin.ts"); const scriptInvalid = resolve("dummy_invalid_plugin.ts"); +const scriptValidDispose = resolve("dummy_valid_dispose_plugin.ts"); +const scriptInvalidDispose = resolve("dummy_invalid_dispose_plugin.ts"); const runtimepathPlugin = resolve("dummy_plugins"); testHost({ @@ -99,9 +101,7 @@ testHost({ await t.step("does not load a denops plugin", async () => { const actual = wait( - async () => - (await host.call("eval", "g:__test_denops_events") as string[]) - .includes("DenopsPluginPost:dummy"), + () => host.call("eval", "len(g:__test_denops_events)"), { timeout: 1000, interval: 100 }, ); await assertRejects(() => actual, Error, "Timeout"); @@ -147,6 +147,133 @@ testHost({ ); }); + await t.step("denops#plugin#unload()", async (t) => { + await t.step("if the plugin is already loaded", async (t) => { + await host.call("execute", [ + "let g:__test_denops_events = []", + `call denops#plugin#load('dummyUnload', '${scriptValidDispose}')`, + ], ""); + await wait(async () => + (await host.call("eval", "g:__test_denops_events") as string[]) + .includes("DenopsPluginPost:dummyUnload") + ); + + outputs = []; + await host.call("execute", [ + "let g:__test_denops_events = []", + "call denops#plugin#unload('dummyUnload')", + ], ""); + + await t.step("unloads a denops plugin", async () => { + await wait(async () => + (await host.call("eval", "g:__test_denops_events") as string[]) + .includes("DenopsPluginUnloadPost:dummyUnload") + ); + }); + + await t.step("fires DenopsPlugin* events", async () => { + assertEquals(await host.call("eval", "g:__test_denops_events"), [ + "DenopsPluginUnloadPre:dummyUnload", + "DenopsPluginUnloadPost:dummyUnload", + ]); + }); + + await t.step("calls the plugin dispose method", () => { + assertMatch(outputs.join(""), /Goodbye, Denops!/); + }); + }); + + await t.step("if the plugin dispose method throws", async (t) => { + await host.call("execute", [ + "let g:__test_denops_events = []", + `call denops#plugin#load('dummyUnloadInvalid', '${scriptInvalidDispose}')`, + ], ""); + await wait(async () => + (await host.call("eval", "g:__test_denops_events") as string[]) + .includes("DenopsPluginPost:dummyUnloadInvalid") + ); + + outputs = []; + await host.call("execute", [ + "let g:__test_denops_events = []", + "call denops#plugin#unload('dummyUnloadInvalid')", + ], ""); + + await t.step("unloads a denops plugin", async () => { + await wait(async () => + (await host.call("eval", "g:__test_denops_events") as string[]) + .includes("DenopsPluginUnloadFail:dummyUnloadInvalid") + ); + }); + + await t.step("fires DenopsPlugin* events", async () => { + assertEquals(await host.call("eval", "g:__test_denops_events"), [ + "DenopsPluginUnloadPre:dummyUnloadInvalid", + "DenopsPluginUnloadFail:dummyUnloadInvalid", + ]); + }); + + await t.step("outputs an error message after delayed", async () => { + await delay(MESSAGE_DELAY); + assertMatch( + outputs.join(""), + /Failed to unload plugin 'dummyUnloadInvalid': Error: This is dummy error in async dispose/, + ); + }); + }); + + await t.step("if the plugin is not yet loaded", async (t) => { + outputs = []; + await host.call("execute", [ + "let g:__test_denops_events = []", + "call denops#plugin#unload('notexistsplugin')", + ], ""); + + await t.step("does not unload a denops plugin", async () => { + const actual = wait( + () => host.call("eval", "len(g:__test_denops_events)"), + { timeout: 1000, interval: 100 }, + ); + await assertRejects(() => actual, Error, "Timeout"); + }); + + await t.step("does not fires DenopsPlugin* events", async () => { + assertEquals(await host.call("eval", "g:__test_denops_events"), []); + }); + + await t.step("does not output messages", async () => { + await delay(MESSAGE_DELAY); + assertEquals(outputs, []); + }); + }); + + // NOTE: Depends on 'dummyUnload' which was already unloaded in the test above. + await t.step("if the plugin is already unloaded", async (t) => { + outputs = []; + await host.call("execute", [ + "let g:__test_denops_events = []", + "call denops#plugin#unload('dummyUnload')", + ], ""); + + await t.step("does not unload a denops plugin", async () => { + const actual = wait( + () => host.call("eval", "len(g:__test_denops_events)"), + { timeout: 1000, interval: 100 }, + ); + await assertRejects(() => actual, Error, "Timeout"); + }); + + await t.step("does not fires DenopsPlugin* events", async () => { + assertEquals(await host.call("eval", "g:__test_denops_events"), []); + }); + + await t.step("does not output messages", async () => { + await delay(MESSAGE_DELAY); + assertEquals(outputs, []); + }); + }); + }); + await t.step("denops#plugin#reload()", async (t) => { // NOTE: Depends on 'dummy' which was already loaded in the test above. await t.step("if the plugin is already loaded", async (t) => { @@ -165,6 +292,8 @@ testHost({ await t.step("fires DenopsPlugin* events", async () => { assertEquals(await host.call("eval", "g:__test_denops_events"), [ + "DenopsPluginUnloadPre:dummy", + "DenopsPluginUnloadPost:dummy", "DenopsPluginPre:dummy", "DenopsPluginPost:dummy", ]); @@ -175,6 +304,47 @@ testHost({ }); }); + await t.step("if the plugin dispose method throws", async (t) => { + await host.call("execute", [ + "let g:__test_denops_events = []", + `call denops#plugin#load('dummyReloadInvalid', '${scriptInvalidDispose}')`, + ], ""); + await wait(async () => + (await host.call("eval", "g:__test_denops_events") as string[]) + .includes("DenopsPluginPost:dummyReloadInvalid") + ); + + outputs = []; + await host.call("execute", [ + "let g:__test_denops_events = []", + "call denops#plugin#reload('dummyReloadInvalid')", + ], ""); + + await t.step("reloads a denops plugin", async () => { + await wait(async () => + (await host.call("eval", "g:__test_denops_events") as string[]) + .includes("DenopsPluginPost:dummyReloadInvalid") + ); + }); + + await t.step("fires DenopsPlugin* events", async () => { + assertEquals(await host.call("eval", "g:__test_denops_events"), [ + "DenopsPluginUnloadPre:dummyReloadInvalid", + "DenopsPluginUnloadFail:dummyReloadInvalid", + "DenopsPluginPre:dummyReloadInvalid", + "DenopsPluginPost:dummyReloadInvalid", + ]); + }); + + await t.step("outputs an error message after delayed", async () => { + await delay(MESSAGE_DELAY); + assertMatch( + outputs.join(""), + /Failed to unload plugin 'dummyReloadInvalid': Error: This is dummy error in async dispose/, + ); + }); + }); + await t.step("if the plugin is not yet loaded", async (t) => { outputs = []; await host.call("execute", [ @@ -184,9 +354,33 @@ testHost({ await t.step("does not reload a denops plugin", async () => { const actual = wait( - async () => - (await host.call("eval", "g:__test_denops_events") as string[]) - .includes("DenopsPluginPost:dummy"), + () => host.call("eval", "len(g:__test_denops_events)"), + { timeout: 1000, interval: 100 }, + ); + await assertRejects(() => actual, Error, "Timeout"); + }); + + await t.step("does not fires DenopsPlugin* events", async () => { + assertEquals(await host.call("eval", "g:__test_denops_events"), []); + }); + + await t.step("does not output messages", async () => { + await delay(MESSAGE_DELAY); + assertEquals(outputs, []); + }); + }); + + // NOTE: Depends on 'dummyUnload' which was already unloaded in the test above. + await t.step("if the plugin is already unloaded", async (t) => { + outputs = []; + await host.call("execute", [ + "let g:__test_denops_events = []", + "call denops#plugin#reload('dummyUnload')", + ], ""); + + await t.step("does not reload a denops plugin", async () => { + const actual = wait( + () => host.call("eval", "len(g:__test_denops_events)"), { timeout: 1000, interval: 100 }, ); await assertRejects(() => actual, Error, "Timeout"); diff --git a/tests/denops/testdata/dummy_invalid_dispose_plugin.ts b/tests/denops/testdata/dummy_invalid_dispose_plugin.ts new file mode 100644 index 00000000..eafad403 --- /dev/null +++ b/tests/denops/testdata/dummy_invalid_dispose_plugin.ts @@ -0,0 +1,11 @@ +// TODO: #349 Import `Entrypoint` from denops-core. +// import type { Entrypoint } from "jsr:@denops/core@7.0.0"; +import type { Entrypoint } from "/denops-private/plugin.ts"; + +export const main: Entrypoint = (_denops) => { + return { + [Symbol.asyncDispose]: () => { + throw new Error("This is dummy error in async dispose"); + }, + }; +}; diff --git a/tests/denops/testdata/dummy_valid_dispose_plugin.ts b/tests/denops/testdata/dummy_valid_dispose_plugin.ts new file mode 100644 index 00000000..65d7aebe --- /dev/null +++ b/tests/denops/testdata/dummy_valid_dispose_plugin.ts @@ -0,0 +1,11 @@ +// TODO: #349 Import `Entrypoint` from denops-core. +// import type { Entrypoint } from "jsr:@denops/core@7.0.0"; +import type { Entrypoint } from "/denops-private/plugin.ts"; + +export const main: Entrypoint = (denops) => { + return { + [Symbol.asyncDispose]: async () => { + await denops.cmd("echo 'Goodbye, Denops!'"); + }, + }; +}; From 3759c616e9e70587a9086083908d23f8e06b9406 Mon Sep 17 00:00:00 2001 From: Milly Date: Wed, 3 Jul 2024 21:03:25 +0900 Subject: [PATCH 2/5] :herb: add script rewrite reload test This breaks `--watch` option. --- deno.jsonc | 2 +- denops/@denops-private/service_test.ts | 92 ++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 1 deletion(-) diff --git a/deno.jsonc b/deno.jsonc index 1e196b23..fe5572d5 100644 --- a/deno.jsonc +++ b/deno.jsonc @@ -3,7 +3,7 @@ "check": "deno check **/*.ts", "test": "LANG=C deno test -A --parallel --shuffle --doc", "test:coverage": "deno task test --coverage=.coverage", - "coverage": "deno coverage .coverage", + "coverage": "deno coverage --exclude=\"test[.]ts(#.*)?$\" .coverage", "update": "deno run --allow-env --allow-read --allow-write=. --allow-run=git,deno --allow-net=jsr.io,registry.npmjs.org jsr:@molt/cli **/*.ts", "update:commit": "deno task -q update --commit --pre-commit=fmt,lint" }, diff --git a/denops/@denops-private/service_test.ts b/denops/@denops-private/service_test.ts index 5a778463..f29bc5e0 100644 --- a/denops/@denops-private/service_test.ts +++ b/denops/@denops-private/service_test.ts @@ -22,6 +22,7 @@ import { promiseState } from "jsr:@lambdalisue/async@2.1.1"; import { unimplemented } from "jsr:@lambdalisue/errorutil@1.0.0"; import type { Host } from "./denops.ts"; import { Service } from "./service.ts"; +import { toFileUrl } from "jsr:@std/path@0.225.0/to-file-url"; const NOOP = () => {}; @@ -641,6 +642,87 @@ Deno.test("Service", async (t) => { assertSpyCalls(host_call, 0); }); }); + + await t.step("if the plugin file is changed", async (t) => { + // Generate source script file. + await using tempFile = await useTempFile({ + // NOTE: Temporary script files should be ignored from coverage. + prefix: "test-denops-service-", + suffix: "_test.ts", + }); + const scriptRewrite = toFileUrl(tempFile.path).href; + const sourceOriginal = await Deno.readTextFile(new URL(scriptValid)); + await Deno.writeTextFile(new URL(scriptRewrite), sourceOriginal); + + const service = new Service(meta); + service.bind(host); + { + using _host_call = stub(host, "call"); + await service.load("dummy", scriptRewrite); + } + using host_call = stub(host, "call"); + + // Change source script file. + const sourceRewrited = sourceOriginal.replaceAll( + "Hello, Denops!", + "Source Changed!", + ); + await Deno.writeTextFile(new URL(scriptRewrite), sourceRewrited); + + await t.step("resolves", async () => { + await service.reload("dummy"); + }); + + await t.step("emits DenopsSystemPluginUnloadPre", () => { + assertSpyCall(host_call, 0, { + args: [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginUnloadPre:dummy", + {}, + ], + }); + }); + + await t.step("emits DenopsSystemPluginUnloadPost", () => { + assertSpyCall(host_call, 1, { + args: [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginUnloadPost:dummy", + {}, + ], + }); + }); + + await t.step("emits DenopsSystemPluginPre", () => { + assertSpyCall(host_call, 2, { + args: [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginPre:dummy", + {}, + ], + }); + }); + + await t.step("calls the plugin entrypoint", () => { + assertSpyCall(host_call, 3, { + args: [ + "denops#api#cmd", + "echo 'Source Changed!'", + {}, + ], + }); + }); + + await t.step("emits DenopsSystemPluginPost", () => { + assertSpyCall(host_call, 4, { + args: [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginPost:dummy", + {}, + ], + }); + }); + }); }); await t.step(".waitLoaded()", async (t) => { @@ -1190,3 +1272,13 @@ Deno.test("Service", async (t) => { function resolve(path: string): string { return new URL(`../../tests/denops/testdata/${path}`, import.meta.url).href; } + +async function useTempFile(options?: Deno.MakeTempOptions) { + const path = await Deno.makeTempFile(options); + return { + path, + async [Symbol.asyncDispose]() { + await Deno.remove(path, { recursive: true }); + }, + }; +} From 9552317062278791931f6b6283888a1d2af838b6 Mon Sep 17 00:00:00 2001 From: Milly Date: Sat, 6 Jul 2024 20:35:09 +0900 Subject: [PATCH 3/5] :herb: improve test coverage --- denops/@denops-private/service_test.ts | 110 +++++++++++++++++++++++++ 1 file changed, 110 insertions(+) diff --git a/denops/@denops-private/service_test.ts b/denops/@denops-private/service_test.ts index f29bc5e0..367bdd1e 100644 --- a/denops/@denops-private/service_test.ts +++ b/denops/@denops-private/service_test.ts @@ -525,6 +525,116 @@ Deno.test("Service", async (t) => { assertSpyCalls(host_call, 0); }); }); + + await t.step("if called before `load()` resolves", async (t) => { + const service = new Service(meta); + service.bind(host); + using host_call = stub(host, "call"); + + const loadPromise = service.load("dummy", scriptValid); + const unloadPromise = service.unload("dummy"); + + await t.step("resolves", async () => { + await unloadPromise; + }); + + await t.step("`load()` was resolved", async () => { + assertEquals(await promiseState(loadPromise), "fulfilled"); + }); + + await t.step("emits events in the correct order", () => { + const events = host_call.calls + .map((c) => c.args) + .filter((args) => (args[1] as string)?.startsWith("doautocmd")); + assertEquals(events, [ + [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginPre:dummy", + {}, + ], + [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginPost:dummy", + {}, + ], + [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginUnloadPre:dummy", + {}, + ], + [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginUnloadPost:dummy", + {}, + ], + ]); + }); + }); + + await t.step("if called before `load()` resolves with error", async (t) => { + const service = new Service(meta); + service.bind(host); + using host_call = stub(host, "call"); + + const loadPromise = service.load("dummy", scriptInvalid); + const unloadPromise = service.unload("dummy"); + + await t.step("resolves", async () => { + await unloadPromise; + }); + + await t.step("`load()` was resolved", async () => { + assertEquals(await promiseState(loadPromise), "fulfilled"); + }); + + await t.step("emits events in the correct order", () => { + const events = host_call.calls + .map((c) => c.args) + .filter((args) => (args[1] as string)?.startsWith("doautocmd")); + assertEquals(events, [ + [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginPre:dummy", + {}, + ], + [ + "denops#api#cmd", + "doautocmd User DenopsSystemPluginFail:dummy", + {}, + ], + ]); + }); + }); + + await t.step("if `host.call()` rejects (channel closed)", async (t) => { + const service = new Service(meta); + service.bind(host); + { + using _host_call = stub(host, "call"); + await service.load("dummy", scriptValid); + } + using console_error = stub(console, "error"); + using _host_call = stub( + host, + "call", + () => Promise.reject(new Error("channel closed")), + ); + + await t.step("resolves", async () => { + await service.unload("dummy"); + }); + + await t.step("outputs error messages", () => { + assertEquals(console_error.calls.map((c) => c.args), [ + [ + "Failed to emit DenopsSystemPluginUnloadPre:dummy: Error: channel closed", + ], + [ + "Failed to emit DenopsSystemPluginUnloadPost:dummy: Error: channel closed", + ], + ]); + }); + }); }); await t.step(".reload()", async (t) => { From 4c197e973e1aae4f0facff9770c083959d6e8a10 Mon Sep 17 00:00:00 2001 From: Milly Date: Sat, 6 Jul 2024 20:37:01 +0900 Subject: [PATCH 4/5] :+1: fix `Service.waitLoaded()` never resolved It is never resolved if it is called between `load()` and `unload()`. --- denops/@denops-private/service.ts | 3 +++ denops/@denops-private/service_test.ts | 28 ++++++++++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/denops/@denops-private/service.ts b/denops/@denops-private/service.ts index 4552a5cd..c992bbbd 100644 --- a/denops/@denops-private/service.ts +++ b/denops/@denops-private/service.ts @@ -73,6 +73,9 @@ export class Service implements HostService, AsyncDisposable { } return; } + this.#waiters.get(name)?.promise.finally(() => { + this.#waiters.delete(name); + }); this.#plugins.delete(name); await plugin.unload(); return plugin; diff --git a/denops/@denops-private/service_test.ts b/denops/@denops-private/service_test.ts index 367bdd1e..30e5e478 100644 --- a/denops/@denops-private/service_test.ts +++ b/denops/@denops-private/service_test.ts @@ -846,6 +846,18 @@ Deno.test("Service", async (t) => { assertEquals(await promiseState(actual), "pending"); }); + await t.step("pendings if the plugin is already unloaded", async () => { + const service = new Service(meta); + service.bind(host); + using _host_call = stub(host, "call"); + await service.load("dummy", scriptValid); + await service.unload("dummy"); + + const actual = service.waitLoaded("dummy"); + + assertEquals(await promiseState(actual), "pending"); + }); + await t.step("resolves if the plugin is already loaded", async () => { const service = new Service(meta); service.bind(host); @@ -868,6 +880,22 @@ Deno.test("Service", async (t) => { assertEquals(await promiseState(actual), "fulfilled"); }); + await t.step( + "resolves if it is called between `load()` and `unload()`", + async () => { + const service = new Service(meta); + service.bind(host); + using _host_call = stub(host, "call"); + + const loadPromise = service.load("dummy", scriptValid); + const actual = service.waitLoaded("dummy"); + const unloadPromise = service.unload("dummy"); + await Promise.all([loadPromise, unloadPromise]); + + assertEquals(await promiseState(actual), "fulfilled"); + }, + ); + await t.step("rejects if the service is already closed", async () => { const service = new Service(meta); service.bind(host); From a519c0f60c87605c82dcebf9734a0ae3483b57c2 Mon Sep 17 00:00:00 2001 From: Milly Date: Sun, 7 Jul 2024 15:32:20 +0900 Subject: [PATCH 5/5] :+1: do not use `Partial` --- denops/@denops-private/service.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/denops/@denops-private/service.ts b/denops/@denops-private/service.ts index c992bbbd..cdd949d7 100644 --- a/denops/@denops-private/service.ts +++ b/denops/@denops-private/service.ts @@ -183,7 +183,7 @@ type PluginModule = { class Plugin { #denops: Denops; #loadedWaiter: Promise; - #disposable: Partial = {}; + #disposable: AsyncDisposable = voidAsyncDisposable; readonly name: string; readonly script: string; @@ -204,7 +204,7 @@ class Plugin { try { await emit(this.#denops, `DenopsSystemPluginPre:${this.name}`); const mod: PluginModule = await import(`${this.script}${suffix}`); - this.#disposable = await mod.main(this.#denops) ?? {}; + this.#disposable = await mod.main(this.#denops) ?? voidAsyncDisposable; await emit(this.#denops, `DenopsSystemPluginPost:${this.name}`); } catch (e) { // Show a warning message when Deno module cache issue is detected @@ -236,13 +236,13 @@ class Plugin { } try { await emit(this.#denops, `DenopsSystemPluginUnloadPre:${this.name}`); - await this.#disposable[Symbol.asyncDispose]?.(); + await this.#disposable[Symbol.asyncDispose](); await emit(this.#denops, `DenopsSystemPluginUnloadPost:${this.name}`); } catch (e) { console.error(`Failed to unload plugin '${this.name}': ${e}`); await emit(this.#denops, `DenopsSystemPluginUnloadFail:${this.name}`); } finally { - this.#disposable = {}; + this.#disposable = voidAsyncDisposable; } } @@ -258,6 +258,10 @@ class Plugin { } } +const voidAsyncDisposable = { + [Symbol.asyncDispose]: () => Promise.resolve(), +} as const satisfies AsyncDisposable; + const loadedScripts = new Set(); function createScriptSuffix(script: string): string {