Skip to content

Conversation

@jlengstorf
Copy link
Contributor

Refactor callback management to use Map so that we can unregister
handlers without changing the current behavior.

Refactor callback management to use `Map` so that we can unregister
handlers without changing the current behavior.

Co-authored-by: Cassidy Williams <[email protected]>
@netlify
Copy link

netlify bot commented Jul 2, 2020

Deploy Success!

Built with commit 61bd509

https://deploy-preview-283--identity.netlify.app

Copy link
Contributor

@mraerino mraerino left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not use a Set? Should do the same right?

@jlengstorf
Copy link
Contributor Author

@mraerino no real reason — we were thinking of key/val when we walked through solutions, and I didn't actually realize that Set had delete available 😂

@cassidoo cassidoo added the type: feature code contributing to the implementation of a feature and/or user facing functionality label Jul 2, 2020
@erezrokah erezrokah self-requested a review July 5, 2020 12:53
@erezrokah
Copy link
Contributor

@jlengstorf, I added some tests and allowed doing .off("login") so the caller doesn't have to maintain a reference to the original callback.

Also handled a case where calling off before on would throw (I think it should be a no-op in that case, we could also throw a designated error).

If we're all good with the changes I think this is ready to merge.

@jlengstorf
Copy link
Contributor Author

thanks for adding those tests, @erezrokah!

Copy link

@philhawksworth philhawksworth left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me

@jlengstorf jlengstorf merged commit 8407a21 into netlify:master Jul 6, 2020
@jlengstorf jlengstorf deleted the feat/unregister-callbacks branch July 6, 2020 17:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

type: feature code contributing to the implementation of a feature and/or user facing functionality

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants