Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

.off behaves incorrectly #22

Closed
sqal opened this issue Oct 5, 2017 · 10 comments
Closed

.off behaves incorrectly #22

sqal opened this issue Oct 5, 2017 · 10 comments

Comments

@sqal
Copy link

sqal commented Oct 5, 2017

Test case:

const emitter = dush()

const handler1 = () => { console.log('a') }
const handler2 = () => { console.log('a') }

emitter.on('a', handler1)
emitter.on('a', handler2)
emitter.off('a', handler1)

What is expected?

handler2 should not be removed

What is actually happening?

handler2 is removed because the lib uses fn.toString() to identify a function and source code of both functions looks the same

@superguigui
Copy link
Contributor

Hi !
I stumbled on this issue as well,
@tunnckoCore would you be interested in a PR for this one ?

@tunnckoCore
Copy link
Member

tunnckoCore commented Nov 14, 2018

That's "known bug", based on how I decided (which is pretty weird and tricky hack) to do the off-ing. It is string based. And if both functions are the same well then yea.

I definitely don't remember why I chose that hack, instead of doing it with few other possible ways.

@superguigui
Copy link
Contributor

superguigui commented Nov 14, 2018

I made some tests and if you remove the string based comparaison and replace it with reference based comparaison, it seems to be ok.

I'm guessing you went with string based because the "off" function was not able to find the wrapped version of the handler. My solution stores a reference to the original handler in the wrapper rather than a string. It's actually very little change and every test is still passing (even losing a couple bytes in the final bundle).

Any other reason you used string based comparaison that i'm not thinking of ?

I have a PR almost ready !

@tunnckoCore
Copy link
Member

tunnckoCore commented Nov 14, 2018

My solution stores a reference to the original handler in the wrapper rather than a string.

I was using this before ;d

But anyway I'm not hard about that, so PR is welcome.

(Oh, I should definitely update the repo boilerplate and stuff (oh god, there's no automatic publish and release here?! so it's really old of haha) ;d But if you are okey with that, great!)

@dwightjack
Copy link

Hi, sorry to bother. I ran into the same problem. Is there any news about a fix release?

@tunnckoCore
Copy link
Member

There's #42 but conflicts should be resolved so I can merge.

@kouts
Copy link

kouts commented Apr 25, 2020

Any news on this @tunnckoCore ?

@tunnckoCore
Copy link
Member

@kouts, sorry, not for now. I can come back in few days and look into an update. I'm switching and upgrading machines now.

@boli-duality
Copy link

@kouts, sorry, not for now. I can come back in few days and look into an update. I'm switching and upgrading machines now.

Hi, just wanna check if there is a update going on. thanks in advanced!

@sqal
Copy link
Author

sqal commented May 9, 2024

@boli-duality I did not expect to receive notifications from this repo ^^. The project is most likely dead, not updated for 7 years. I would advise you to simply find another event emitter. mitt is fine.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants