From 8f6236965cd20b9272167bc6829276fc692b2cda Mon Sep 17 00:00:00 2001 From: Shigma Date: Thu, 26 May 2022 23:09:06 +0800 Subject: [PATCH] feat(cli): support reusable plugin reload --- build/dtsc.ts | 3 +- package.json | 8 +-- packages/cli/src/worker/watcher.ts | 64 ++++++++++++--------- packages/core/package.json | 2 +- packages/core/tests/command.spec.ts | 32 ++++++----- packages/core/tests/middleware.spec.ts | 3 +- packages/core/tests/selector.spec.ts | 3 +- packages/core/tests/session.spec.ts | 6 +- packages/koishi/src/patch.ts | 5 +- packages/utils/src/misc.ts | 2 +- plugins/a11y/schedule/tests/index.spec.ts | 3 +- plugins/common/repeater/tests/index.spec.ts | 3 +- 12 files changed, 77 insertions(+), 57 deletions(-) diff --git a/build/dtsc.ts b/build/dtsc.ts index cab309d43e..39f5389901 100644 --- a/build/dtsc.ts +++ b/build/dtsc.ts @@ -43,9 +43,10 @@ async function bundleNodes(nodes: Node[]) { for (const node of nodes) { await fs.mkdir(resolve(cwd, node.path, 'lib'), { recursive: true }) console.log('building', node.path) - await spawnAsync(['yarn', 'dtsc'], { + const code = await spawnAsync(['yarn', 'dtsc'], { cwd: resolve(cwd, node.path), }) + if (code) process.exit(code) } } diff --git a/package.json b/package.json index 9b00209927..d1a7190c4b 100644 --- a/package.json +++ b/package.json @@ -88,13 +88,13 @@ "typescript": "^4.7.2", "yakumo": "^0.2.4", "yakumo-mocha": "^0.2.4", - "yakumo-publish": "^0.2.2", - "yakumo-upgrade": "^0.2.2", - "yakumo-version": "^0.2.3" + "yakumo-publish": "^0.2.3", + "yakumo-upgrade": "^0.2.3", + "yakumo-version": "^0.2.4" }, "yakumo": { "require": [ "esbuild-register" ] } -} +} \ No newline at end of file diff --git a/packages/cli/src/worker/watcher.ts b/packages/cli/src/worker/watcher.ts index 3804d9f86f..c55986f95c 100644 --- a/packages/cli/src/worker/watcher.ts +++ b/packages/cli/src/worker/watcher.ts @@ -203,27 +203,27 @@ class Watcher { this.analyzeChanges() /** plugins pending classification */ - const pending = new Map() + const pending = new Map() /** plugins that should be reloaded */ - const reloads = new Map() + const reloads = new Map() // we assume that plugin entry files are "atomic" // that is, reloading them will not cause any other reloads for (const filename in require.cache) { const module = require.cache[filename] const plugin = ns.unwrapExports(module.exports) - const state = this.ctx.app.registry.get(plugin) - if (!state || this.declined.has(filename)) continue - pending.set(filename, state) + const runtime = this.ctx.app.registry.get(plugin) + if (!runtime || this.declined.has(filename)) continue + pending.set(filename, runtime) if (!plugin['sideEffect']) this.declined.add(filename) } - for (const [filename, state] of pending) { + for (const [filename, runtime] of pending) { // check if it is a dependent of the changed file this.declined.delete(filename) const dependencies = [...loadDependencies(filename, this.declined)] - if (!state.plugin['sideEffect']) this.declined.add(filename) + if (!runtime.plugin['sideEffect']) this.declined.add(filename) // we only detect reloads at plugin level // a plugin will be reloaded if any of its dependencies are accepted @@ -231,21 +231,38 @@ class Watcher { dependencies.forEach(dep => this.accepted.add(dep)) // prepare for reload - let ancestor = state, isMarked = false - while ((ancestor = ancestor.parent?.state) && !(isMarked = reloads.has(ancestor))); - if (!isMarked) reloads.set(state, filename) + let isMarked = false + const visited = new Set() + const queued = [runtime] + while (queued.length) { + const runtime = queued.shift() + if (visited.has(runtime)) continue + visited.add(runtime) + if (reloads.has(runtime)) { + isMarked = true + break + } + for (const state of runtime.children) { + queued.push(state.runtime) + } + } + if (!isMarked) reloads.set(runtime, filename) } // save require.cache for rollback + // and delete module cache before re-require const backup: Dict = {} for (const filename of this.accepted) { backup[filename] = require.cache[filename] + delete require.cache[filename] } - // delete module cache before re-require - this.accepted.forEach((path) => { - delete require.cache[path] - }) + /** rollback require.cache */ + function rollback() { + for (const filename in backup) { + require.cache[filename] = backup[filename] + } + } // attempt to load entry files const attempts = {} @@ -254,30 +271,25 @@ class Watcher { attempts[filename] = ns.unwrapExports(require(filename)) } } catch (err) { - // rollback require.cache logger.warn(err) return rollback() } - function rollback() { - for (const filename in backup) { - require.cache[filename] = backup[filename] - } - } - try { - for (const [state, filename] of reloads) { + for (const [runtime, filename] of reloads) { const path = relative(this.root, filename) try { - this.ctx.dispose(state.plugin) + this.ctx.dispose(runtime.plugin) } catch (err) { logger.warn('failed to dispose plugin at %c\n' + coerce(err), path) } try { const plugin = attempts[filename] - state.parent.plugin(plugin, state.config) + for (const state of runtime.children) { + state.parent.plugin(plugin, state.config) + } logger.info('reload plugin at %c', path) } catch (err) { logger.warn('failed to reload plugin at %c\n' + coerce(err), path) @@ -287,10 +299,10 @@ class Watcher { } catch { // rollback require.cache and plugin states rollback() - for (const [state, filename] of reloads) { + for (const [runtime, filename] of reloads) { try { this.ctx.dispose(attempts[filename]) - state.parent.plugin(state.plugin, state.config) + runtime.parent.plugin(runtime.plugin, runtime.config) } catch (err) { logger.warn(err) } diff --git a/packages/core/package.json b/packages/core/package.json index 672f4f3074..8512240e55 100644 --- a/packages/core/package.json +++ b/packages/core/package.json @@ -36,7 +36,7 @@ }, "dependencies": { "@koishijs/utils": "^5.4.5", - "cordis": "^1.2.0", + "cordis": "^1.2.1", "fastest-levenshtein": "^1.0.12", "minato": "^1.1.0" } diff --git a/packages/core/tests/command.spec.ts b/packages/core/tests/command.spec.ts index 95ca34e4bc..4bd2ddc739 100644 --- a/packages/core/tests/command.spec.ts +++ b/packages/core/tests/command.spec.ts @@ -157,27 +157,28 @@ describe('Command API', () => { }) it('patch command', () => { - app.plugin((ctx) => { + const dispose = app.plugin((ctx) => { ctx.command('foo', { patch: true }).alias('fooo').option('opt', 'option 1') ctx.command('abc', { patch: true }).alias('abcd').option('opt', 'option 1') - - const foo = app.$commander._commands.get('foo') - expect(foo).to.be.ok - expect(app.$commander._commands.get('fooo')).to.be.ok - expect(Object.keys(foo._options)).to.have.length(2) - expect(app.$commander._commands.get('abc')).to.be.undefined - expect(app.$commander._commands.get('abcd')).to.be.undefined - - ctx.dispose() - expect(app.$commander._commands.get('foo')).to.be.ok - expect(app.$commander._commands.get('fooo')).to.be.undefined - expect(Object.keys(foo._options)).to.have.length(1) }) + + const foo = app.$commander._commands.get('foo') + expect(foo).to.be.ok + expect(app.$commander._commands.get('fooo')).to.be.ok + expect(Object.keys(foo._options)).to.have.length(2) + expect(app.$commander._commands.get('abc')).to.be.undefined + expect(app.$commander._commands.get('abcd')).to.be.undefined + + dispose() + expect(app.$commander._commands.get('foo')).to.be.ok + expect(app.$commander._commands.get('fooo')).to.be.undefined + expect(Object.keys(foo._options)).to.have.length(1) }) }) describe('Execute Commands', () => { - const app = new App().plugin(mock) + const app = new App() + app.plugin(mock) const session = app.mock.session({}) const warn = jest.spyOn(logger, 'warn') const next = jest.fn(Next.compose) @@ -293,7 +294,8 @@ describe('Command API', () => { }) describe('Bypass Middleware', async () => { - const app = new App().plugin(mock) + const app = new App() + app.plugin(mock) const client = app.mock.client('123') app.middleware((session, next) => { diff --git a/packages/core/tests/middleware.spec.ts b/packages/core/tests/middleware.spec.ts index ee33d6f43f..8cb39ab368 100644 --- a/packages/core/tests/middleware.spec.ts +++ b/packages/core/tests/middleware.spec.ts @@ -3,7 +3,8 @@ import { expect } from 'chai' import mock from '@koishijs/plugin-mock' import * as jest from 'jest-mock' -const app = new App().plugin(mock) +const app = new App() +app.plugin(mock) const midLogger = new Logger('session') const midWarn = jest.spyOn(midLogger, 'warn') diff --git a/packages/core/tests/selector.spec.ts b/packages/core/tests/selector.spec.ts index 7fb79aec1e..792cb6635f 100644 --- a/packages/core/tests/selector.spec.ts +++ b/packages/core/tests/selector.spec.ts @@ -2,7 +2,8 @@ import { App } from 'koishi' import { expect } from 'chai' import mock from '@koishijs/plugin-mock' -const app = new App().plugin(mock) +const app = new App() +app.plugin(mock) const guildSession = app.mock.session({ userId: '123', guildId: '456', subtype: 'group' }) const privateSession = app.mock.session({ userId: '123', subtype: 'private' }) diff --git a/packages/core/tests/session.spec.ts b/packages/core/tests/session.spec.ts index 67755ce26b..5e74aa21cd 100644 --- a/packages/core/tests/session.spec.ts +++ b/packages/core/tests/session.spec.ts @@ -3,7 +3,8 @@ import mock from '@koishijs/plugin-mock' describe('Session API', () => { describe('Command Execution', () => { - const app = new App().plugin(mock) + const app = new App() + app.plugin(mock) const client = app.mock.client('456') app.command('echo [content:text]').action((_, text) => text) @@ -30,7 +31,8 @@ describe('Session API', () => { }) describe('Other Session Methods', () => { - const app = new App({ prefix: '.' }).plugin(mock) + const app = new App({ prefix: '.' }) + app.plugin(mock) const client = app.mock.client('123', '456') before(() => app.start()) diff --git a/packages/koishi/src/patch.ts b/packages/koishi/src/patch.ts index e18486de76..1b0cb9581f 100644 --- a/packages/koishi/src/patch.ts +++ b/packages/koishi/src/patch.ts @@ -10,7 +10,7 @@ declare module '@koishijs/core' { namespace Registry { interface Delegates { - plugin(path: string, config?: any): Context + plugin(path: string, config?: any): () => boolean } } } @@ -38,8 +38,7 @@ Context.prototype.plugin = function (this: Context, entry: any, config?: any) { if (typeof entry === 'string') { entry = scope.require(entry) } - plugin.call(this, entry, config) - return this + return plugin.call(this, entry, config) } const start = App.prototype.start diff --git a/packages/utils/src/misc.ts b/packages/utils/src/misc.ts index 30fcb60171..dd2c532486 100644 --- a/packages/utils/src/misc.ts +++ b/packages/utils/src/misc.ts @@ -25,7 +25,7 @@ export function merge(head: T, base: T): T { return head } -export function assertProperty(config: O, key: K) { +export function assertProperty(config: O, key: K) { if (!config[key]) throw new Error(`missing configuration "${key}"`) return config[key] } diff --git a/plugins/a11y/schedule/tests/index.spec.ts b/plugins/a11y/schedule/tests/index.spec.ts index 07751b8906..77c3c855e2 100644 --- a/plugins/a11y/schedule/tests/index.spec.ts +++ b/plugins/a11y/schedule/tests/index.spec.ts @@ -7,7 +7,8 @@ import { expect } from 'chai' import 'chai-shape' describe('@koishijs/plugin-switch', () => { - const app = new App().plugin(mock) + const app = new App() + app.plugin(mock) const client1 = app.mock.client('123', '456') const client2 = app.mock.client('123') diff --git a/plugins/common/repeater/tests/index.spec.ts b/plugins/common/repeater/tests/index.spec.ts index e91dc19fc5..df1e411ace 100644 --- a/plugins/common/repeater/tests/index.spec.ts +++ b/plugins/common/repeater/tests/index.spec.ts @@ -3,7 +3,8 @@ import mock from '@koishijs/plugin-mock' import * as repeater from '@koishijs/plugin-repeater' async function setup(config: repeater.Config) { - const app = new App().plugin(mock) + const app = new App() + app.plugin(mock) const client1 = app.mock.client('123', '123') const client2 = app.mock.client('456', '123') const client3 = app.mock.client('789', '123')