fix(loader): fix entry garbage collection

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<symbol> = 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,13 +113,18 @@ 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]) {
         ctx.root[symbol2] = ctx.root[symbol1]
         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)

packages/loader/src/shared.ts

@@ -110,6 +110,8 @@ export abstract class Loader<T extends Loader.Options = Loader.Options> 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<T extends Loader.Options = Loader.Options> 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<T extends Loader.Options = Loader.Options> extends
     if (sourceEntry === targetEntry) return
     entry.parent = targetEntry.fork.ctx
     if (!entry.fork) return
-    entry.amend()
+    entry.patch()
   }
 
   paths(scope: EffectScope): string[] {

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,

packages/loader/tests/utils.ts

@@ -7,7 +7,6 @@ import { expect } from 'chai'
 declare module '../src/shared' {
   interface Loader {
     mock<F extends Function>(name: string, plugin: F): Mock<F>
-    restart(config: Entry.Options[]): Promise<void>
     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