Meteor-Community-Packages / meteor-collection-hooks

Meteor Collection Hooks
https://atmospherejs.com/matb33/collection-hooks
MIT License
656 stars 90 forks source link

hook "remove" does not remove hook or removes wrong hook #268

Closed erikholleboom closed 1 year ago

erikholleboom commented 3 years ago

To repoduce:

  1. Add first hook (before.update)
  2. Add second hook (before.update)
  3. remove first hook
  4. remove second hook
  5. update a entry --> second hook is called.

Problem is in de splice call "len": remove () { self._hookAspects[method][pointcut].splice(len - 1, 1)

Len was stored when inserting the hook in the array. However due to the removal of the first hook, the second hook became first in the list. Therefore it will not get removed.

Simple node example showing the issue:

n=[] [] n.push(1) -> add first hook 1 -> internally stored len n.push(2) -> add second hook 2 -> internally stored len n --> self._hookAspects[method][pointcut] [ 1, 2 ] n.splice(0,1) --> len-1, 1 [ 1 ] -> 1 is removed n [ 2 ] n.splice(1,1) --> len-1, 1 [] -> nothing is removed n -> second hook still in the list... [ 2 ]

Actually the fun start when adding and removing hooks in various orders then one hook actually removed the wrong hook! which is causing me to not use remove any longer, only use replace! which is a safe workaround if it works for your application.

StorytellerCZ commented 3 years ago

Hi @erikholleboom , thank you for the report! Since you already have a good understanding of the bug would you be interested in creating a fix for it?