From 643f4b19a898785d73d7feaf6fa0aa5ca2f609c6 Mon Sep 17 00:00:00 2001 From: Shigma Date: Wed, 29 May 2024 22:18:03 +0800 Subject: [PATCH] feat(loader): refactor entry algorithm - tree.update() and tree.transfer() all in one - do not fork.update() when config does not change --- packages/loader/src/entry.ts | 87 +++++++++++++++------------ packages/loader/src/group.ts | 12 ++-- packages/loader/src/loader.ts | 8 --- packages/loader/src/tree.ts | 45 ++++++-------- packages/loader/tests/isolate.spec.ts | 8 +-- packages/loader/tests/utils.ts | 2 +- 6 files changed, 81 insertions(+), 81 deletions(-) diff --git a/packages/loader/src/entry.ts b/packages/loader/src/entry.ts index 9e75c91..ff6c024 100644 --- a/packages/loader/src/entry.ts +++ b/packages/loader/src/entry.ts @@ -14,7 +14,6 @@ export namespace Entry { intercept?: Dict | null isolate?: Dict | null inject?: string[] | Inject | null - when?: any } } @@ -63,20 +62,24 @@ export class Entry { } } - patch(ctx: Context, ref: Context = ctx) { - // part 1: prepare isolate map - const newMap: Dict = Object.create(Object.getPrototypeOf(ref[Context.isolate])) + patch(options: Partial = {}) { + // step 1: prepare isolate map + const ctx = this.fork?.parent ?? this.parent.ctx.extend({ + [Context.intercept]: Object.create(this.parent.ctx[Context.intercept]), + [Context.isolate]: Object.create(this.parent.ctx[Context.isolate]), + }) + const newMap: Dict = Object.create(this.parent.ctx[Context.isolate]) for (const [key, label] of Object.entries(this.options.isolate ?? {})) { const realm = this.resolveRealm(label) newMap[key] = (this.loader.realms[realm] ??= Object.create(null))[key] ??= Symbol(`${key}${realm}`) } - // part 2: generate service diff + // step 2: generate service diff const diff: [string, symbol, symbol, symbol, symbol][] = [] const oldMap = ctx[Context.isolate] for (const key in { ...oldMap, ...newMap, ...this.loader.delims }) { if (newMap[key] === oldMap[key]) continue - const delim = this.loader.delims[key] ??= Symbol(key) + const delim = this.loader.delims[key] ??= Symbol(`delim:${key}`) ctx[delim] = Symbol(`${key}#${this.options.id}`) for (const symbol of [oldMap[key], newMap[key]]) { const value = symbol && ctx[symbol] @@ -91,8 +94,7 @@ export class Entry { } } - // part 3: emit service events - // part 3.1: internal/before-service + // step 3: emit internal/before-service for (const [key, symbol1, symbol2, flag1, flag2] of diff) { const self = Object.create(ctx) self[Context.filter] = (target: Context) => { @@ -102,17 +104,21 @@ export class Entry { ctx.emit(self, 'internal/before-service', key) } - // part 3.2: update service impl - if (ctx === ref) { - swap(ctx[Context.isolate], newMap) - swap(ctx[Context.intercept], this.options.intercept) - // prevent double update - this.fork?.update(this.options.config) - } else { - // handle entry transfer - Object.setPrototypeOf(ctx, Object.getPrototypeOf(ref)) - swap(ctx, ref) + // step 4: update + // step 4.1: patch context + Object.setPrototypeOf(ctx, this.parent.ctx) + Object.setPrototypeOf(ctx[Context.isolate], this.parent.ctx[Context.isolate]) + Object.setPrototypeOf(ctx[Context.intercept], this.parent.ctx[Context.intercept]) + swap(ctx[Context.isolate], newMap) + swap(ctx[Context.intercept], this.options.intercept) + + // step 4.2: update fork when options.config is updated + if (this.fork && 'config' in options) { + this.suspend = true + this.fork.update(this.options.config) } + + // step 4.3: update service impl for (const [, symbol1, symbol2, flag1, flag2] of diff) { if (flag1 === flag2 && ctx[symbol1] && !ctx[symbol2]) { ctx.root[symbol2] = ctx.root[symbol1] @@ -120,7 +126,7 @@ export class Entry { } } - // part 3.3: internal/service + // step 5: emit internal/service for (const [key, symbol1, symbol2, flag1, flag2] of diff) { const self = Object.create(ctx) self[Context.filter] = (target: Context) => { @@ -130,19 +136,14 @@ export class Entry { ctx.emit(self, 'internal/service', key) } - // part 4: clean up delimiter + // step 6: clean up delimiters for (const key in this.loader.delims) { if (!Reflect.ownKeys(newMap).includes(key)) { delete ctx[this.loader.delims[key]] } } - } - createContext() { - return this.parent.ctx.extend({ - [Context.intercept]: Object.create(this.parent.ctx[Context.intercept]), - [Context.isolate]: Object.create(this.parent.ctx[Context.isolate]), - }) + return ctx } get requiredInjects() { @@ -161,7 +162,6 @@ export class Entry { } _check() { - if (!this.loader.isTruthyLike(this.options.when)) return false if (this.options.disabled) return false for (const name of this.requiredInjects) { let key = this.parent.ctx[Context.isolate][name] @@ -185,33 +185,46 @@ export class Entry { } } - async update(options: Entry.Options) { - const legacy = this.options - this.options = sortKeys(options) + async update(options: Partial, override = false) { + const legacy = { ...this.options } + + // step 1: update options + if (override) { + this.options = options as Entry.Options + } else { + for (const [key, value] of Object.entries(options)) { + if (isNullable(value)) { + delete this.options[key] + } else { + this.options[key] = value + } + } + } + sortKeys(this.options) + + // step 2: execute if (!this._check()) { await this.stop() } else if (this.fork) { - this.suspend = true for (const [key, label] of Object.entries(legacy.isolate ?? {})) { if (this.options.isolate?.[key] === label) continue const name = this.resolveRealm(label) this.loader._clearRealm(key, name) } - this.patch(this.fork.parent) + this.patch(options) } else { await this.start() } } async start() { - const ctx = this.createContext() - const exports = await this.loader.import(this.options.name, this.parent.tree.url).catch((error: any) => { - ctx.emit('internal/error', new Error(`Cannot find package "${this.options.name}"`)) - ctx.emit('internal/error', error) + const exports = await this.parent.tree.import(this.options.name).catch((error: any) => { + this.parent.ctx.emit('internal/error', new Error(`Cannot find package "${this.options.name}"`)) + this.parent.ctx.emit('internal/error', error) }) if (!exports) return const plugin = this.loader.unwrapExports(exports) - this.patch(ctx) + const ctx = this.patch() ctx[Entry.key] = this this.fork = ctx.plugin(plugin, this.options.config) ctx.emit('loader/entry', 'apply', this) diff --git a/packages/loader/src/group.ts b/packages/loader/src/group.ts index 7f000da..05160d7 100644 --- a/packages/loader/src/group.ts +++ b/packages/loader/src/group.ts @@ -11,10 +11,12 @@ export class EntryGroup { } async create(options: Omit) { - const id = this.ctx.loader.ensureId(options) - const entry = this.ctx.loader.entries[id] ??= new Entry(this.ctx.loader, this) + const id = this.tree.ensureId(options) + const entry = this.tree.entries[id] ??= new Entry(this.ctx.loader, this) + // Entry may be moved from another group, + // so we need to update the parent reference. entry.parent = this - await entry.update(options as Entry.Options) + await entry.update(options as Entry.Options, true) return id } @@ -25,11 +27,11 @@ export class EntryGroup { } remove(id: string) { - const entry = this.ctx.loader.entries[id] + const entry = this.tree.entries[id] if (!entry) return entry.stop() this.unlink(entry.options) - delete this.ctx.loader.entries[id] + delete this.tree.entries[id] } update(config: Entry.Options[]) { diff --git a/packages/loader/src/loader.ts b/packages/loader/src/loader.ts index a680138..9db650c 100644 --- a/packages/loader/src/loader.ts +++ b/packages/loader/src/loader.ts @@ -164,14 +164,6 @@ export abstract class Loader extends ImportTree { return this._locate(scope.parent.scope) } - async import(name: string, baseURL = this.url) { - if (this.internal) { - return this.internal.import(name, baseURL, {}) - } else { - return import(name) - } - } - exit() {} unwrapExports(exports: any) { diff --git a/packages/loader/src/tree.ts b/packages/loader/src/tree.ts index 74154dd..cba2b1a 100644 --- a/packages/loader/src/tree.ts +++ b/packages/loader/src/tree.ts @@ -1,5 +1,5 @@ import { Context } from '@cordisjs/core' -import { Dict, isNullable } from 'cosmokit' +import { Dict } from 'cosmokit' import { Entry } from './entry.ts' import { EntryGroup } from './group.ts' @@ -23,21 +23,6 @@ export abstract class EntryTree { return options.id! } - async update(id: string, options: Partial>) { - const entry = this.entries[id] - if (!entry) throw new Error(`entry ${id} not found`) - const override = { ...entry.options } - for (const [key, value] of Object.entries(options)) { - if (isNullable(value)) { - delete override[key] - } else { - override[key] = value - } - } - entry.parent.tree.write() - return entry.update(override) - } - resolveGroup(id: string | null) { const group = id ? this.entries[id]?.subgroup : this.root if (!group) throw new Error(`entry ${id} not found`) @@ -58,20 +43,28 @@ export abstract class EntryTree { entry.parent.tree.write() } - transfer(id: string, parent: string | null, position = Infinity) { + async update(id: string, options: Omit, parent?: string | null, position?: number) { const entry = this.entries[id] if (!entry) throw new Error(`entry ${id} not found`) const source = entry.parent - const target = this.resolveGroup(parent) - source.unlink(entry.options) - target.data.splice(position, 0, entry.options) source.tree.write() - target.tree.write() - if (source === target) return - entry.parent = target - if (!entry.fork) return - const ctx = entry.createContext() - entry.patch(entry.fork.parent, ctx) + let target: EntryGroup | undefined + if (parent !== undefined) { + target = this.resolveGroup(parent) + source.unlink(entry.options) + target.data.splice(position ?? Infinity, 0, entry.options) + target.tree.write() + entry.parent = target + } + return entry.update(options) + } + + async import(name: string) { + if (this.ctx.loader.internal) { + return this.ctx.loader.internal.import(name, this.url, {}) + } else { + return import(name) + } } abstract write(): void diff --git a/packages/loader/tests/isolate.spec.ts b/packages/loader/tests/isolate.spec.ts index 7bc4425..5e8f50b 100644 --- a/packages/loader/tests/isolate.spec.ts +++ b/packages/loader/tests/isolate.spec.ts @@ -514,7 +514,7 @@ describe('service isolation: transfer', () => { }) it('transfer injector into group', async () => { - loader.transfer(injector, group) + loader.update(injector, {}, group) await new Promise((resolve) => setTimeout(resolve, 0)) expect(foo.mock.calls).to.have.length(0) @@ -522,7 +522,7 @@ describe('service isolation: transfer', () => { }) it('transfer provider into group', async () => { - loader.transfer(provider, group) + loader.update(provider, {}, group) await new Promise((resolve) => setTimeout(resolve, 0)) expect(foo.mock.calls).to.have.length(1) @@ -530,7 +530,7 @@ describe('service isolation: transfer', () => { }) it('transfer injector out of group', async () => { - loader.transfer(injector, null) + loader.update(injector, {}, null) await new Promise((resolve) => setTimeout(resolve, 0)) expect(foo.mock.calls).to.have.length(0) @@ -538,7 +538,7 @@ describe('service isolation: transfer', () => { }) it('transfer provider out of group', async () => { - loader.transfer(provider, null) + loader.update(provider, {}, null) await new Promise((resolve) => setTimeout(resolve, 0)) expect(foo.mock.calls).to.have.length(1) diff --git a/packages/loader/tests/utils.ts b/packages/loader/tests/utils.ts index 6b218fb..5e344c7 100644 --- a/packages/loader/tests/utils.ts +++ b/packages/loader/tests/utils.ts @@ -4,7 +4,7 @@ import { LoaderFile, Entry, Group, Loader } from '../src' import { Mock, mock } from 'node:test' import { expect } from 'chai' -declare module '../src/shared' { +declare module '../src/index.ts' { interface Loader { mock(name: string, plugin: F): Mock expectEnable(plugin: any, config?: any): void