From c9be0aa1f98904ecda93ffee44d69eb1653efdd1 Mon Sep 17 00:00:00 2001 From: Shigma Date: Thu, 6 Jun 2024 18:46:47 +0800 Subject: [PATCH] feat(cordis): service method will no longer trace inject from plugins --- packages/core/src/context.ts | 5 +++-- packages/core/src/events.ts | 14 ++++++++------ packages/core/src/registry.ts | 27 +++++++++++++++++---------- packages/core/src/scope.ts | 9 ++++----- packages/core/src/service.ts | 1 + packages/core/src/utils.ts | 6 +++++- packages/core/tests/invoke.spec.ts | 3 ++- packages/core/tests/service.spec.ts | 26 +++++++++++++------------- packages/core/tests/update.spec.ts | 17 ----------------- packages/core/tests/utils.ts | 17 +++++++++++++++-- packages/loader/src/inject.ts | 2 ++ packages/loader/src/isolate.ts | 2 ++ 12 files changed, 72 insertions(+), 57 deletions(-) diff --git a/packages/core/src/context.ts b/packages/core/src/context.ts index a327f0f..ad1efda 100644 --- a/packages/core/src/context.ts +++ b/packages/core/src/context.ts @@ -104,8 +104,9 @@ export class Context { // Case 4: access directly from root if (!ctx.runtime.plugin) return // Case 5: custom inject checks - if (ctx.bail('internal/inject', name)) return - ctx.emit(ctx, 'internal/warning', new Error(`property ${name} is not registered, declare it as \`inject\` to suppress this warning`)) + if (ctx.bail(ctx, 'internal/inject', name)) return + const warning = new Error(`property ${name} is not registered, declare it as \`inject\` to suppress this warning`) + ctx.emit(ctx, 'internal/warning', warning) } const [name, internal] = Context.resolveInject(ctx, prop) diff --git a/packages/core/src/events.ts b/packages/core/src/events.ts index e115049..5f7221e 100644 --- a/packages/core/src/events.ts +++ b/packages/core/src/events.ts @@ -97,13 +97,15 @@ export class Lifecycle { }, { global: true }), Context.static, ctx.scope) // inject in ancestor contexts - defineProperty(this.on('internal/inject', function (name) { - let parent = this - while (parent.runtime.plugin) { - for (const key of parent.runtime.inject) { - if (name === Context.resolveInject(parent, key)[0]) return true + defineProperty(this.on('internal/inject', function (this: Context, name) { + let ctx = this + while (ctx !== ctx.root) { + if (Reflect.ownKeys(ctx).includes('scope')) { + for (const key of ctx.runtime.inject) { + if (name === Context.resolveInject(ctx, key)[0]) return true + } } - parent = parent.scope.parent + ctx = ctx[symbols.trace] ?? Object.getPrototypeOf(ctx) } }, { global: true }), Context.static, ctx.scope) } diff --git a/packages/core/src/registry.ts b/packages/core/src/registry.ts index 0c363b5..b4ac94c 100644 --- a/packages/core/src/registry.ts +++ b/packages/core/src/registry.ts @@ -64,11 +64,15 @@ declare module './context.ts' { export class Registry { private _counter = 0 private _internal = new Map>() + protected context: Context - constructor(private ctx: Context, config: any) { + constructor(public ctx: C, config: any) { defineProperty(this, Context.origin, ctx) - ctx.scope = new MainScope(this, null!, config) - ctx.scope.runtime.isReactive = true + this.context = ctx + const runtime = new MainScope(ctx, null!, config) + ctx.scope = runtime + runtime.ctx = ctx + this.set(null!, runtime) } get counter() { @@ -139,15 +143,17 @@ export class Registry { // check if it's a valid plugin this.resolve(plugin, true) - const context: Context = this.ctx - context.scope.assertActive() + // magic: this.ctx[symbols.trace] === this + // Here we ignore the reference + const ctx: C = Object.getPrototypeOf(this.ctx) + ctx.scope.assertActive() // resolve plugin config let error: any try { config = resolveConfig(plugin, config) } catch (reason) { - context.emit('internal/error', reason) + this.context.emit(ctx, 'internal/error', reason) error = reason config = null } @@ -156,12 +162,13 @@ export class Registry { let runtime = this.get(plugin) if (runtime) { if (!runtime.isForkable) { - context.emit('internal/warning', new Error(`duplicate plugin detected: ${plugin.name}`)) + this.context.emit(ctx, 'internal/warning', new Error(`duplicate plugin detected: ${plugin.name}`)) } - return runtime.fork(context, config, error) + return runtime.fork(ctx, config, error) } - runtime = new MainScope(this, plugin, config, error) - return runtime.fork(context, config, error) + runtime = new MainScope(ctx, plugin, config, error) + this.set(plugin, runtime) + return runtime.fork(ctx, config, error) } } diff --git a/packages/core/src/scope.ts b/packages/core/src/scope.ts index 06b89ce..50dfd31 100644 --- a/packages/core/src/scope.ts +++ b/packages/core/src/scope.ts @@ -1,6 +1,6 @@ import { deepEqual, defineProperty, isNullable, remove } from 'cosmokit' import { Context } from './context.ts' -import { Inject, Plugin, Registry } from './registry.ts' +import { Inject, Plugin } from './registry.ts' import { isConstructor, resolveConfig } from './utils.ts' declare module './context.ts' { @@ -167,7 +167,7 @@ export abstract class EffectScope { } get ready() { - return this.runtime.using.every(name => !isNullable(this.ctx[name])) + return this.runtime.using.every(name => !isNullable(this.ctx.get(name))) } reset() { @@ -310,9 +310,8 @@ export class MainScope extends EffectScope { isReusable?: boolean = false isReactive?: boolean = false - constructor(registry: Registry, public plugin: Plugin, config: any, error?: any) { - super(registry[Context.origin] as C, config) - registry.set(plugin, this) + constructor(ctx: C, public plugin: Plugin, config: any, error?: any) { + super(ctx, config) if (!plugin) { this.name = 'root' this.isActive = true diff --git a/packages/core/src/service.ts b/packages/core/src/service.ts index 73a1dfc..b845891 100644 --- a/packages/core/src/service.ts +++ b/packages/core/src/service.ts @@ -5,6 +5,7 @@ import { Spread } from './registry.ts' export abstract class Service { static readonly setup: unique symbol = symbols.setup as any + static readonly trace: unique symbol = symbols.trace as any static readonly invoke: unique symbol = symbols.invoke as any static readonly extend: unique symbol = symbols.extend as any static readonly provide: unique symbol = symbols.provide as any diff --git a/packages/core/src/utils.ts b/packages/core/src/utils.ts index 540d7e7..8c89ce6 100644 --- a/packages/core/src/utils.ts +++ b/packages/core/src/utils.ts @@ -14,6 +14,7 @@ export const symbols = { // service symbols setup: Symbol.for('cordis.setup') as typeof Service.setup, + trace: Symbol.for('cordis.trace') as typeof Service.trace, invoke: Symbol.for('cordis.invoke') as typeof Service.invoke, extend: Symbol.for('cordis.extend') as typeof Service.extend, provide: Symbol.for('cordis.provide') as typeof Service.provide, @@ -57,7 +58,10 @@ export function joinPrototype(proto1: {}, proto2: {}) { export function createTraceable(ctx: any, value: any) { const proxy = new Proxy(value, { get: (target, name, receiver) => { - if (name === symbols.origin || name === 'ctx') return ctx + if (name === symbols.origin || name === 'ctx') { + const origin = Reflect.getOwnPropertyDescriptor(target, symbols.origin)?.value + return ctx.extend({ [symbols.trace]: origin }) + } return Reflect.get(target, name, receiver) }, apply: (target, thisArg, args) => { diff --git a/packages/core/tests/invoke.spec.ts b/packages/core/tests/invoke.spec.ts index 87e2774..8a0a359 100644 --- a/packages/core/tests/invoke.spec.ts +++ b/packages/core/tests/invoke.spec.ts @@ -1,5 +1,6 @@ import { expect } from 'chai' import { Context, Service } from '../src' +import './utils' describe('functional service', () => { it('functional service', async () => { @@ -14,7 +15,7 @@ describe('functional service', () => { static [Service.immediate] = true protected [Service.invoke](init?: Config) { - const caller = this[Context.origin] + const caller = this.ctx expect(caller).to.be.instanceof(Context) let result = { ...this.config } let intercept = caller[Context.intercept] diff --git a/packages/core/tests/service.spec.ts b/packages/core/tests/service.spec.ts index 3dc741c..4ab931f 100644 --- a/packages/core/tests/service.spec.ts +++ b/packages/core/tests/service.spec.ts @@ -2,7 +2,7 @@ import { Context, Service } from '../src' import { noop } from 'cosmokit' import { expect } from 'chai' import { mock } from 'node:test' -import { checkError, getHookSnapshot } from './utils' +import { checkError, Counter, getHookSnapshot } from './utils' describe('Service', () => { it('non-service access', async () => { @@ -128,36 +128,36 @@ describe('Service', () => { expect(callback.mock.calls).to.have.length(1) }) - it('traceable effect', async () => { + it('traceable effect (inject)', async () => { class Foo extends Service { - size = 0 + static inject = ['counter'] constructor(ctx: Context) { super(ctx, 'foo', true) } - increse() { - return this.ctx.effect(() => { - this.size++ - return () => this.size-- - }) + count() { + this.ctx.counter.increse() + return this.ctx.counter.value } } const root = new Context() const warning = mock.fn() root.on('internal/warning', warning) + root.set('counter', new Counter(root)) + root.plugin(Foo) - root.foo.increse() - expect(root.foo.size).to.equal(1) + expect(root.foo.count()).to.equal(1) + expect(root.foo.count()).to.equal(2) const fork = root.inject(['foo'], (ctx) => { - ctx.foo.increse() - expect(ctx.foo.size).to.equal(2) + expect(ctx.foo.count()).to.equal(3) + expect(ctx.foo.count()).to.equal(4) }) fork.dispose() - expect(root.foo.size).to.equal(1) + expect(root.foo.count()).to.equal(3) expect(warning.mock.calls).to.have.length(0) }) diff --git a/packages/core/tests/update.spec.ts b/packages/core/tests/update.spec.ts index 9da7c27..aa14ab2 100644 --- a/packages/core/tests/update.spec.ts +++ b/packages/core/tests/update.spec.ts @@ -204,21 +204,4 @@ describe('Update', () => { expect(fork.disposables).to.have.length(1) // effect expect(fork.runtime.disposables).to.have.length(1) // fork }) - - it('root update', async () => { - const root = new Context() - const callback = mock.fn(noop) - const { length } = root.state.disposables - - root.decline(['foo']) - root.on('dispose', callback) - expect(callback.mock.calls).to.have.length(0) - - root.state.update({ maxListeners: 100 }) - expect(callback.mock.calls).to.have.length(0) - - root.state.update({ foo: 100 }) - expect(callback.mock.calls).to.have.length(1) - expect(root.state.disposables.length).to.equal(length) - }) }) diff --git a/packages/core/tests/utils.ts b/packages/core/tests/utils.ts index e79964d..6b08205 100644 --- a/packages/core/tests/utils.ts +++ b/packages/core/tests/utils.ts @@ -28,8 +28,7 @@ export class Filter { export function filter(ctx: Context) { ctx.root.filter = () => true ctx.on('internal/runtime', (runtime) => { - // same as `!runtime.uid`, but to make coverage happy - if (!ctx.registry.has(runtime.plugin)) return + if (!runtime.uid) return runtime.ctx.filter = (session) => { return runtime.children.some((child) => { return child.ctx.filter(session) @@ -44,11 +43,25 @@ declare module '../src/events' { } } +export class Counter { + value = 0 + + constructor(public ctx: Context) {} + + increse() { + return this.ctx.effect(() => { + this.value++ + return () => this.value-- + }) + } +} + declare module '../src/context' { interface Context { foo: any bar: any baz: any + counter: Counter filter(session: Session): boolean } diff --git a/packages/loader/src/inject.ts b/packages/loader/src/inject.ts index f40e608..925cace 100644 --- a/packages/loader/src/inject.ts +++ b/packages/loader/src/inject.ts @@ -7,6 +7,8 @@ declare module './entry.ts' { } } +export const name = 'inject' + export function apply(ctx: Context) { function getRequired(entry?: Entry) { return Array.isArray(entry?.options.inject) diff --git a/packages/loader/src/isolate.ts b/packages/loader/src/isolate.ts index fcc3a9c..dbeeaa4 100644 --- a/packages/loader/src/isolate.ts +++ b/packages/loader/src/isolate.ts @@ -69,6 +69,8 @@ export class GlobalRealm extends Realm { } } +export const name = 'isolate' + export function apply(ctx: Context) { const realms: Dict = Object.create(null)