From fdf20959e90c4c4af7b0453bab2856785ecb8685 Mon Sep 17 00:00:00 2001 From: Sergii Date: Mon, 16 Aug 2021 23:39:26 +0300 Subject: [PATCH] fix(ability): ensure ability call all event handlers during emit Event when they are unsunscribed in emit phase --- packages/casl-ability/spec/ability.spec.js | 50 +++++++++++++------ packages/casl-ability/src/RuleIndex.ts | 21 +++++--- .../casl-ability/src/structures/LinkedItem.ts | 14 ++++-- 3 files changed, 60 insertions(+), 25 deletions(-) diff --git a/packages/casl-ability/spec/ability.spec.js b/packages/casl-ability/spec/ability.spec.js index ed86966ef..7afc97680 100755 --- a/packages/casl-ability/spec/ability.spec.js +++ b/packages/casl-ability/spec/ability.spec.js @@ -157,7 +157,7 @@ describe('Ability', () => { expect(ability).not.to.allow('publish', 'Post') }) - describe('`update` method', () => { + describe('`update` method and events', () => { let updateHandler beforeEach(() => { @@ -201,24 +201,46 @@ describe('Ability', () => { expect(anotherHandler).to.have.been.called() }) - it('invokes all subscribers even if they are changed during "emit" phase', () => { - const firstSubscription = setupListenerChangesInListener() - const secondSubscription = setupListenerChangesInListener() + it('can unregister itself inside event handler', () => { + const handlers = [spy(), spy()] + const unsubscribe1 = ability.on('updated', () => { + handlers[0]() + unsubscribe1() + }) + ability.on('updated', handlers[1]) ability.update([]) - expect(firstSubscription).to.have.been.called() - expect(secondSubscription).to.have.been.called() - }) + expect(handlers[0]).to.have.been.called() + expect(handlers[1]).to.have.been.called() + }) + + it('can unregister another event handler inside own handler', () => { + let results = [] + const handlers = [ + spy(() => results.push(0)), + spy(() => results.push(1)), + spy(() => results.push(2)), + spy(() => results.push(3)) + ] + const unsubscribe = [] + + unsubscribe[0] = ability.on('updated', handlers[0]) + unsubscribe[1] = ability.on('updated', handlers[1]) + unsubscribe[2] = ability.on('updated', handlers[2]) + unsubscribe[3] = ability.on('updated', () => { + handlers[3]() + unsubscribe[2]() + }) + ability.update([]) + + expect(results).to.deep.equal([3, 2, 1, 0]) - function setupListenerChangesInListener() { - const unsubscribe = spy(ability.on('update', function listen() { - unsubscribe() - ability.on('update', listen) - })) + results = [] + ability.update([{ action: 'read', subject: 'all' }]) - return unsubscribe - } + expect(results).to.deep.equal([3, 1, 0]) + }) }) }) diff --git a/packages/casl-ability/src/RuleIndex.ts b/packages/casl-ability/src/RuleIndex.ts index 8de9fa4c5..3126111b5 100644 --- a/packages/casl-ability/src/RuleIndex.ts +++ b/packages/casl-ability/src/RuleIndex.ts @@ -9,7 +9,7 @@ import { ExtractSubjectType } from './types'; import { wrapArray, detectSubjectType, mergePrioritized, getOrDefault, identity, isSubjectType } from './utils'; -import { LinkedItem, linkedItem, unlinkItem } from './structures/LinkedItem'; +import { LinkedItem, linkedItem, unlinkItem, cloneLinkedItem } from './structures/LinkedItem'; export interface RuleIndexOptions extends Partial> { detectSubjectType?( @@ -48,16 +48,20 @@ interface AbilityEvent { export interface UpdateEvent extends AbilityEvent { rules: RawRuleOf[] } +/** + * @deprecated `on`/`emit` properly infer type without this type + * TODO(major): delete + */ export type EventHandler = (event: Event) => void; export type Events< T extends WithGenerics, K extends keyof EventsMap = keyof EventsMap -> = Map[K]>> | null>; +> = Map[K]> | null>; interface EventsMap { - update: UpdateEvent - updated: UpdateEvent + update(event: UpdateEvent): void + updated(event: UpdateEvent): void } type IndexTree = Map { on>( event: T, - handler: EventHandler>[T]> + handler: EventsMap>[T] ): Unsubscribe { const head = this._events.get(event) || null; const item = linkedItem(handler, head); @@ -227,10 +231,13 @@ export class RuleIndex { }; } - private _emit>(name: T, payload: EventsMap[T]) { + private _emit>( + name: T, + payload: Parameters[T]>[0] + ) { let current = this._events.get(name) || null; while (current !== null) { - const prev = current.prev; + const prev = current.prev ? cloneLinkedItem(current.prev) : null; current.value(payload); current = prev; } diff --git a/packages/casl-ability/src/structures/LinkedItem.ts b/packages/casl-ability/src/structures/LinkedItem.ts index d90856418..201e7f302 100644 --- a/packages/casl-ability/src/structures/LinkedItem.ts +++ b/packages/casl-ability/src/structures/LinkedItem.ts @@ -4,7 +4,7 @@ export interface LinkedItem { readonly value: T } -export const linkedItem = (value: T, prev: LinkedItem['prev']) => { +export function linkedItem(value: T, prev: LinkedItem['prev']) { const item = { value, prev, next: null }; if (prev) { @@ -12,9 +12,9 @@ export const linkedItem = (value: T, prev: LinkedItem['prev']) => { } return item; -}; +} -export const unlinkItem = (item: LinkedItem) => { +export function unlinkItem(item: LinkedItem) { if (item.next) { item.next.prev = item.prev; } @@ -24,4 +24,10 @@ export const unlinkItem = (item: LinkedItem) => { } item.next = item.prev = null; // eslint-disable-line -}; +} + +export const cloneLinkedItem = >(item: T): T => ({ + value: item.value, + prev: item.prev, + next: item.next, +} as T);