Skip to content

Commit

Permalink
fix(core): fix effect disposal from outer context, fix #8
Browse files Browse the repository at this point in the history
  • Loading branch information
shigma committed Feb 7, 2024
1 parent fd408a4 commit 957e74d
Show file tree
Hide file tree
Showing 2 changed files with 54 additions and 34 deletions.
10 changes: 8 additions & 2 deletions packages/core/src/scope.ts
Expand Up @@ -97,10 +97,13 @@ export abstract class EffectScope<C extends Context = Context> {
// eslint-disable-next-line new-cap
? new callback(this.ctx, config)
: callback(this.ctx, config)
let disposed = false
const original = typeof result === 'function' ? result : result.dispose.bind(result)
const wrapped = () => {
// make sure the original callback is not called twice
if (!remove(this.disposables, wrapped)) return
if (disposed) return
disposed = true
remove(this.disposables, wrapped)
return original()
}
this.disposables.push(wrapped)
Expand Down Expand Up @@ -313,6 +316,8 @@ export class ForkScope<C extends Context = Context> extends EffectScope<C> {
}

export class MainScope<C extends Context = Context> extends EffectScope<C> {
public value: any

runtime = this
schema: any
name?: string
Expand Down Expand Up @@ -386,6 +391,7 @@ export class MainScope<C extends Context = Context> extends EffectScope<C> {
if (instance['fork']) {
this.forkables.push(instance['fork'].bind(instance))
}
return instance
} else {
return this.plugin(context, config)
}
Expand All @@ -401,7 +407,7 @@ export class MainScope<C extends Context = Context> extends EffectScope<C> {
start() {
if (super.start()) return true
if (!this.isReusable && this.plugin) {
this.ensure(async () => this.apply(this.ctx, this._config))
this.ensure(async () => this.value = this.apply(this.ctx, this._config))
}
for (const fork of this.children) {
fork.start()
Expand Down
78 changes: 46 additions & 32 deletions packages/core/tests/dispose.spec.ts
Expand Up @@ -116,39 +116,53 @@ describe('Disposables', () => {
expect(root.state.disposables.length).to.equal(length)
})

test('ctx.effect()', async () => {
const root = new Context()
const dispose = mock.fn(noop)
const items: Item[] = []

class Item {
constructor() {
items.push(this)
}

dispose() {
dispose()
remove(items, this)
describe('ctx.effect()', () => {
test('manual dispose', async () => {
const root = new Context()
const dispose = mock.fn(noop)
const items: Item[] = []

class Item {
constructor() {
items.push(this)
}

dispose() {
dispose()
remove(items, this)
}
}
}

const item1 = root.effect(() => new Item())
const item2 = root.effect(() => new Item())
expect(item1).instanceof(Item)
expect(item2).instanceof(Item)
expect(dispose.mock.calls).to.have.length(0)
expect(items).to.have.length(2)

item1.dispose()
expect(dispose.mock.calls).to.have.length(1)
expect(items).to.have.length(1)

item1.dispose()
expect(dispose.mock.calls).to.have.length(1)
expect(items).to.have.length(1)

const item1 = root.effect(() => new Item())
const item2 = root.effect(() => new Item())
expect(item1).instanceof(Item)
expect(item2).instanceof(Item)
expect(dispose.mock.calls).to.have.length(0)
expect(items).to.have.length(2)

item1.dispose()
expect(dispose.mock.calls).to.have.length(1)
expect(items).to.have.length(1)

item1.dispose()
expect(dispose.mock.calls).to.have.length(1)
expect(items).to.have.length(1)

item2.dispose()
expect(dispose.mock.calls).to.have.length(2)
expect(items).to.have.length(0)
})

item2.dispose()
expect(dispose.mock.calls).to.have.length(2)
expect(items).to.have.length(0)
test('plugin dispose', () => {
const root = new Context()
const dispose = mock.fn(noop)

const fork = root.plugin((ctx: Context) => {
ctx.effect(() => dispose)
})

fork.dispose()
expect(dispose.mock.calls).to.have.length(1)
})
})
})

0 comments on commit 957e74d

Please sign in to comment.