Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

👍 add plugin unload feature #385

Merged
merged 5 commits into from
Jul 7, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
35 changes: 35 additions & 0 deletions autoload/denops/_internal/plugin.vim
Original file line number Diff line number Diff line change
@@ -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 = {}
Expand All @@ -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)
Expand Down Expand Up @@ -60,10 +69,36 @@ function! s:DenopsSystemPluginFail() abort
execute printf('doautocmd <nomodeline> User DenopsPluginFail:%s', l:name)
endfunction

function! s:DenopsSystemPluginUnloadPre() abort
const l:name = matchstr(expand('<amatch>'), 'DenopsSystemPluginUnloadPre:\zs.*')
let l:plugin = denops#_internal#plugin#get(l:name)
let l:plugin.state = s:STATE_UNLOADING
execute printf('doautocmd <nomodeline> User DenopsPluginUnloadPre:%s', l:name)
endfunction

function! s:DenopsSystemPluginUnloadPost() abort
const l:name = matchstr(expand('<amatch>'), '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 <nomodeline> User DenopsPluginUnloadPost:%s', l:name)
endfunction

function! s:DenopsSystemPluginUnloadFail() abort
const l:name = matchstr(expand('<amatch>'), '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 <nomodeline> 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
4 changes: 4 additions & 0 deletions autoload/denops/plugin.vim
Original file line number Diff line number Diff line change
Expand Up @@ -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
Milly marked this conversation as resolved.
Show resolved Hide resolved

function! denops#plugin#reload(name) abort
call denops#_internal#plugin#reload(a:name)
endfunction
Expand Down
6 changes: 6 additions & 0 deletions deno.jsonc
Original file line number Diff line number Diff line change
Expand Up @@ -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/"
],
Expand Down
4 changes: 4 additions & 0 deletions denops/@denops-private/host.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,7 @@ export type CallbackId = string;
export type Service = {
bind(host: Host): void;
load(name: string, script: string): Promise<void>;
unload(name: string): Promise<void>;
reload(name: string): Promise<void>;
interrupt(reason?: unknown): void;
dispatch(name: string, fn: string, args: unknown[]): Promise<unknown>;
Expand All @@ -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":
Expand All @@ -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),
Expand Down
1 change: 1 addition & 0 deletions denops/@denops-private/host/nvim_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ Deno.test("Neovim", async (t) => {
const service: Service = {
bind: () => unimplemented(),
load: () => unimplemented(),
unload: () => unimplemented(),
reload: () => unimplemented(),
interrupt: () => unimplemented(),
dispatch: () => unimplemented(),
Expand Down
1 change: 1 addition & 0 deletions denops/@denops-private/host/vim_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ Deno.test("Vim", async (t) => {
const service: Service = {
bind: () => unimplemented(),
load: () => unimplemented(),
unload: () => unimplemented(),
reload: () => unimplemented(),
interrupt: () => unimplemented(),
dispatch: () => unimplemented(),
Expand Down
1 change: 1 addition & 0 deletions denops/@denops-private/host_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import { invoke, type Service } from "./host.ts";
Deno.test("invoke", async (t) => {
const service: Omit<Service, "bind"> = {
load: () => unimplemented(),
unload: () => unimplemented(),
reload: () => unimplemented(),
interrupt: () => unimplemented(),
dispatch: () => unimplemented(),
Expand Down
35 changes: 35 additions & 0 deletions denops/@denops-private/plugin.ts
Original file line number Diff line number Diff line change
@@ -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/[email protected]";

/**
* 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<void | AsyncDisposable>;
Comment on lines +33 to +35
Copy link

Choose a reason for hiding this comment

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

Avoid using void in union types.

Using void in union types can be confusing. Replace void with undefined for clarity.

-export type Entrypoint = (
-  denops: Denops,
-) => void | AsyncDisposable | Promise<void | AsyncDisposable>;
+export type Entrypoint = (
+  denops: Denops,
+) => undefined | AsyncDisposable | Promise<undefined | AsyncDisposable>;
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
export type Entrypoint = (
denops: Denops,
) => void | AsyncDisposable | Promise<void | AsyncDisposable>;
export type Entrypoint = (
denops: Denops,
) => undefined | AsyncDisposable | Promise<undefined | AsyncDisposable>;
Tools
Biome

[error] 35-35: void is confusing inside a union type.

Unsafe fix: Use undefined instead.

(lint/suspicious/noConfusingVoidType)

95 changes: 75 additions & 20 deletions denops/@denops-private/service.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,6 @@
// TODO: #349 Import `Entrypoint` from denops-core.
// import type { Entrypoint } from "jsr:@denops/[email protected]";
import type { Entrypoint } from "./plugin.ts";
import type { Denops, Meta } from "jsr:@denops/[email protected]";
import { toFileUrl } from "jsr:@std/[email protected]/to-file-url";
import { toErrorObject } from "jsr:@lambdalisue/[email protected]";
Expand Down Expand Up @@ -38,11 +41,7 @@ export class Service implements HostService, AsyncDisposable {
this.#host = host;
}

async load(
name: string,
script: string,
suffix = "",
): Promise<void> {
async load(name: string, script: string): Promise<void> {
if (this.#closed) {
throw new Error("Service closed");
}
Expand All @@ -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<void> {
async #unload(name: string): Promise<Plugin | undefined> {
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<void> {
await this.#unload(name);
}

async reload(name: string): Promise<void> {
const plugin = await this.#unload(name);
if (plugin) {
await this.load(name, plugin.script);
}
}

waitLoaded(name: string): Promise<void> {
Expand Down Expand Up @@ -137,14 +146,17 @@ export class Service implements HostService, AsyncDisposable {
}
}

close(): Promise<void> {
async close(): Promise<void> {
if (!this.#closed) {
this.#closed = true;
const error = new Error("Service closed");
for (const { reject } of this.#waiters.values()) {
reject(error);
}
this.#waiters.clear();
await Promise.all(
[...this.#plugins.values()].map((plugin) => plugin.unload()),
);
this.#plugins.clear();
this.#host = undefined;
this.#closedWaiter.resolve();
Expand All @@ -161,8 +173,14 @@ export class Service implements HostService, AsyncDisposable {
}
}

type PluginModule = {
main: Entrypoint;
};

class Plugin {
#denops: Denops;
#loadedWaiter: Promise<void>;
#disposable: Partial<AsyncDisposable> = {};
Milly marked this conversation as resolved.
Show resolved Hide resolved

readonly name: string;
readonly script: string;
Expand All @@ -171,13 +189,19 @@ class Plugin {
this.#denops = denops;
this.name = name;
this.script = resolveScriptUrl(script);
this.#loadedWaiter = this.#load();
}

async load(suffix = ""): Promise<void> {
waitLoaded(): Promise<void> {
return this.#loadedWaiter;
}

async #load(): Promise<void> {
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) ?? {};
Milly marked this conversation as resolved.
Show resolved Hide resolved
await emit(this.#denops, `DenopsSystemPluginPost:${this.name}`);
} catch (e) {
// Show a warning message when Deno module cache issue is detected
Expand All @@ -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<void> {
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 = {};
Milly marked this conversation as resolved.
Show resolved Hide resolved
}
}

Expand All @@ -210,6 +255,16 @@ class Plugin {
}
}

const loadedScripts = new Set<string>();

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<void> {
try {
await denops.cmd(`doautocmd <nomodeline> User ${name}`);
Expand Down
Loading