From b5b92f4f08311717fd22686776ac3d6cdb692fc5 Mon Sep 17 00:00:00 2001 From: Shigma Date: Thu, 2 May 2024 00:41:32 +0800 Subject: [PATCH] fix(loader): fix entry garbage collection --- packages/loader/src/entry.ts | 20 ++++++++++++--- packages/loader/src/shared.ts | 6 ++++- packages/loader/tests/index.spec.ts | 40 ++++++++++++++++------------- packages/loader/tests/utils.ts | 6 ----- 4 files changed, 43 insertions(+), 29 deletions(-) diff --git a/packages/loader/src/entry.ts b/packages/loader/src/entry.ts index dd51b99..0d230da 100644 --- a/packages/loader/src/entry.ts +++ b/packages/loader/src/entry.ts @@ -69,16 +69,20 @@ export class Entry { }) ctx.emit('loader/patch', this, legacy) swap(ctx[Context.intercept], this.options.intercept) + + // part 1: prepare isolate map const newMap: Dict = Object.create(Object.getPrototypeOf(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}`) } - for (const key in legacy?.isolate ?? {}) { - if (this.options.isolate?.[key] === legacy!.isolate![key]) continue - const name = this.resolveRealm(legacy!.isolate![key]) + 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) } + + // part 2: generate service diff const delimiter = Symbol('delimiter') ctx[delimiter] = true const diff: [string, symbol, symbol, boolean][] = [] @@ -97,6 +101,9 @@ export class Entry { break } } + + // part 3: emit service events + // part 3.1: internal/before-service for (const [key, symbol1, symbol2, flag] of diff) { const self = Object.create(null) self[Context.filter] = (target: Context) => { @@ -106,6 +113,9 @@ export class Entry { } ctx.emit(self, 'internal/before-service', key) } + + // part 3.2: update service impl, prevent double update + this.fork?.update(this.options.config) swap(ctx[Context.isolate], newMap) for (const [, symbol1, symbol2, flag] of diff) { if (flag && ctx[symbol1] && !ctx[symbol2]) { @@ -113,6 +123,8 @@ export class Entry { delete ctx.root[symbol1] } } + + // part 3.3: internal/service for (const [key, symbol1, symbol2, flag] of diff) { const self = Object.create(null) self[Context.filter] = (target: Context) => { @@ -122,6 +134,7 @@ export class Entry { } ctx.emit(self, 'internal/service', key) } + delete ctx[delimiter] return ctx } @@ -135,7 +148,6 @@ export class Entry { } else if (this.fork) { this.isUpdate = true this.patch(this.fork.parent, legacy) - this.fork.update(this.options.config) } else { this.parent.emit('loader/entry', 'apply', this) const plugin = await this.loader.resolve(this.options.name) diff --git a/packages/loader/src/shared.ts b/packages/loader/src/shared.ts index be6b20d..982e7c0 100644 --- a/packages/loader/src/shared.ts +++ b/packages/loader/src/shared.ts @@ -110,6 +110,8 @@ export abstract class Loader extends if (!this.app.registry.has(fork.runtime.plugin)) return fork.parent.emit('loader/entry', 'unload', fork.entry) fork.entry.options.disabled = true + fork.entry.fork = undefined + fork.entry.stop() this.writeConfig() }) @@ -268,6 +270,8 @@ export abstract class Loader extends const entry = this.entries[id] if (!entry) return entry.stop() + entry.unlink() + delete this.entries[id] } remove(id: string) { @@ -291,7 +295,7 @@ export abstract class Loader extends if (sourceEntry === targetEntry) return entry.parent = targetEntry.fork.ctx if (!entry.fork) return - entry.amend() + entry.patch() } paths(scope: EffectScope): string[] { diff --git a/packages/loader/tests/index.spec.ts b/packages/loader/tests/index.spec.ts index c423a2f..88d8bf1 100644 --- a/packages/loader/tests/index.spec.ts +++ b/packages/loader/tests/index.spec.ts @@ -5,13 +5,16 @@ import MockLoader from './utils' describe('basic support', () => { const root = new Context() root.plugin(MockLoader) + const loader = root.loader - const foo = root.loader.mock('foo', (ctx: Context) => ctx.accept()) - const bar = root.loader.mock('bar', (ctx: Context) => ctx.accept()) - const qux = root.loader.mock('qux', (ctx: Context) => ctx.accept()) + const foo = loader.mock('foo', (ctx: Context) => ctx.accept()) + const bar = loader.mock('bar', (ctx: Context) => ctx.accept()) + const qux = loader.mock('qux', (ctx: Context) => ctx.accept()) + + before(() => loader.start()) it('loader initiate', async () => { - await root.loader.restart([{ + loader.config = [{ id: '1', name: 'foo', }, { @@ -28,11 +31,12 @@ describe('basic support', () => { name: 'qux', disabled: true, }], - }]) + }] + await loader.start() - root.loader.expectEnable(foo, {}) - root.loader.expectEnable(bar, { a: 1 }) - root.loader.expectDisable(qux) + loader.expectEnable(foo, {}) + loader.expectEnable(bar, { a: 1 }) + loader.expectDisable(qux) expect(foo.mock.calls).to.have.length(1) expect(bar.mock.calls).to.have.length(1) expect(qux.mock.calls).to.have.length(0) @@ -41,18 +45,18 @@ describe('basic support', () => { it('loader update', async () => { foo.mock.resetCalls() bar.mock.resetCalls() - root.loader.restart([{ + loader.config = [{ id: '1', name: 'foo', }, { id: '4', name: 'qux', - }]) + }] + await loader.start() - await new Promise((resolve) => setTimeout(resolve, 0)) - root.loader.expectEnable(foo, {}) - root.loader.expectDisable(bar) - root.loader.expectEnable(qux, {}) + loader.expectEnable(foo, {}) + loader.expectDisable(bar) + loader.expectEnable(qux, {}) expect(foo.mock.calls).to.have.length(0) expect(bar.mock.calls).to.have.length(0) expect(qux.mock.calls).to.have.length(1) @@ -61,7 +65,7 @@ describe('basic support', () => { it('plugin self-update 1', async () => { root.registry.get(foo)!.update({ a: 3 }) await new Promise((resolve) => setTimeout(resolve, 0)) - expect(root.loader.config).to.deep.equal([{ + expect(loader.config).to.deep.equal([{ id: '1', name: 'foo', config: { a: 3 }, @@ -74,7 +78,7 @@ describe('basic support', () => { it('plugin self-update 2', async () => { root.registry.get(foo)!.children[0].update({ a: 5 }) await new Promise((resolve) => setTimeout(resolve, 0)) - expect(root.loader.config).to.deep.equal([{ + expect(loader.config).to.deep.equal([{ id: '1', name: 'foo', config: { a: 5 }, @@ -87,7 +91,7 @@ describe('basic support', () => { it('plugin self-dispose 1', async () => { root.registry.get(foo)!.dispose() await new Promise((resolve) => setTimeout(resolve, 0)) - expect(root.loader.config).to.deep.equal([{ + expect(loader.config).to.deep.equal([{ id: '1', name: 'foo', disabled: true, @@ -101,7 +105,7 @@ describe('basic support', () => { it('plugin self-dispose 2', async () => { root.registry.get(qux)!.children[0].dispose() await new Promise((resolve) => setTimeout(resolve, 0)) - expect(root.loader.config).to.deep.equal([{ + expect(loader.config).to.deep.equal([{ id: '1', name: 'foo', disabled: true, diff --git a/packages/loader/tests/utils.ts b/packages/loader/tests/utils.ts index dac669a..ffa9919 100644 --- a/packages/loader/tests/utils.ts +++ b/packages/loader/tests/utils.ts @@ -7,7 +7,6 @@ import { expect } from 'chai' declare module '../src/shared' { interface Loader { mock(name: string, plugin: F): Mock - restart(config: Entry.Options[]): Promise expectEnable(plugin: any, config?: any): void expectDisable(plugin: any): void } @@ -35,11 +34,6 @@ export default class MockLoader extends Loader { return this.config } - async restart(config: Entry.Options[]) { - this.config = config - return this.start() - } - expectEnable(plugin: any, config?: any) { const runtime = this.ctx.registry.get(plugin) expect(runtime).to.be.ok