technomancy / robert-hooke

Hooke your Clojure functions!
Other
358 stars 27 forks source link

Ordering is not guaranteed on hook execution #14

Open jstoneham opened 11 years ago

jstoneham commented 11 years ago

I expected hooks to wrap each other (i.e., to be executed in the opposite order that they were added). They don't - because they're added to a hash-map, they are executed in whichever order 'vals' returns them in. It would be useful to use a map instead that remembered insertion order and work in reverse to get this behavior.

technomancy commented 11 years ago

I sort of agree, but part of using hooks is that you are asserting control over something you don't normally control. This means you need to write your hook in a way that works without assumptions around order.

That said, this isn't always feasible, and I'd accept a patch to change it to preseve insertion order.

jstoneham commented 11 years ago

I'll see if I can come up with one.

As an aside, the context this came up in was using lein-package, where I had the default lein-package hook installed that intercepts lein's install and deploy stuff. In my case, I wanted to apply my hook around that in a certain scope-limited context, but found their hook was still being called first.

kindlychung commented 9 years ago

I have sent a PR to fix this, please check it out.

gfredericks commented 9 years ago

FYI my hook order got rearranged after upgrading clojure 1.6=>1.7.

gfredericks commented 9 years ago

This issue should probably be closed in light of https://github.com/technomancy/robert-hooke/pull/21#issuecomment-102785184, though perhaps we ought to have documentation about intended usage so the issue doesn't pop back up periodically.

marcomorain commented 9 years ago

We've hit this issue at CircleCI when testing for upgrades to Clojure 1.7 (which changes the sort order or hashes).