Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings

[TS] Return unsub() from on() #138

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

Open
developit wants to merge 1 commit into main
base: main
Choose a base branch
Loading
from on-unsub-redo
Open

[TS] Return unsub() from on() #138

developit wants to merge 1 commit into main from on-unsub-redo

Conversation

@developit
Copy link
Owner

@developit developit commented Jun 23, 2021

This is another take on the proposed behavior from #42, where on() returns a function that can be called to remove the added listener.

My concerns remain, however, and I don't think this should be landed. Any implementation that provides this convenience makes it easier to create memory leaks, because there are now two separate owners of a given handler reference - Mitt itself, and the app code that added the listener.

saas786 reacted with eyes emoji
*/
off<Key extends keyof Events>(type: Key, handler?: GenericEventHandler) {
const handlers: Array<GenericEventHandler> | undefined = all!.get(type);
let handlers: Array<GenericEventHandler> | undefined = all!.get(type);
Copy link
Owner Author

@developit developit Jun 23, 2021
edited
Loading

Choose a reason for hiding this comment

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

Note: this is a non-functional change - it just slightly improves compression gains by making the structure of off() match that of on().

Copy link

Size Change: +44 B (+5%) 🔍

Total Size: 1 kB

Filename Size Change
dist/mitt.es.js 231 B +12 B (+5%) 🔍
dist/mitt.js 230 B +12 B (+6%) 🔍
dist/mitt.modern.js 231 B +8 B (+4%)
dist/mitt.umd.js 308 B +12 B (+4%)

compressed-size-action

Copy link

y-nk commented Jul 13, 2021
edited
Loading

@developit i think the feature would find itself very convenient if you consider:

1. wrapping many events in some qualified functions:

function someSpecificFunction() {
 const handler = () => console.log(arguments)
 const unsubs = ['event1', 'event2', 'event3'].map(ev => {
 mitt.on(ev, handler)
 return () => mitt.off(ev, handler)
 })
 return () => unsubs.forEach(fn => fn())
}

which, with this PR could be made more concise like:

function someSpecificFunction() {
 const handler = () => console.log(arguments)
 const unsubs = [
 mitt.on('event1', handler),
 mitt.on('event2', handler),
 mitt.on('event3', handler),
 ]
 return () => unsubs.forEach(fn => fn())
}

2. use in modern frontend libraries such as React:

A typical use could be:

useEffect(() => {
 const listener = mitt.on('event', setStateSomething)
 return listener
}, [someDep])

which, with this PR could be made more concise like:

useEffect(
 () => mitt.on('event', setStateSomething),
 [someDep],
)

In the end people could just also have a workaround such as:

const on = mitt.on
mitt.on = (ev, fn) => on(ev, fn) ?? () => mitt.off(ev, fn)

...but it's such a common use nowadays that it just doesn't make sense not to have it. (i personally look forward to this PR to start using mitt into my company's sdk)

budimirbudimir, tobyzerner, uptonking, Yukiniro, bboydflo, daolanfler, dkern, and ashrafnazar reacted with thumbs up emoji

Copy link

da1z commented Dec 20, 2024

@developit hi. this looks what I am looking for. are you still not considering merging it?

Copy link

royalrex commented Feb 5, 2025

Hi - I'm also looking for this feature. Obviously I will create it in my app for now, but would be nice for the lib to have as a core feature.

About the ownership comment above #138 (comment)
Has it been considered using WeakMap or an implementation using Symbols (stored in a metadata structure) to map registered handler functions with an off function? That way, at least in my head, it could be possible to create off functions without a direct reference to the handler function, and thus avoid memory leaks due to referenced off functions. What impact it will have on the lib size is probably another story.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@jf3888 jf3888 jf3888 approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

AltStyle によって変換されたページ (->オリジナル) /