Skip to content

Fix typing #191

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

Merged
merged 4 commits into from
Feb 8, 2025
Merged

Fix typing #191

merged 4 commits into from
Feb 8, 2025

Conversation

barbieri
Copy link
Contributor

@barbieri barbieri commented Jan 2, 2025

Following some fixes from #190, this time let's use tsc to force us to be in sync.

The previous attempt to manually sync missed some details, then remove
the outdated @types/socket-io and use the built-in types instead.

Then do a mapping of the original IoSocket type to the wrapped,
forcing the wrapper to be returned for chained, properties should
alway match.

This will force the wrapper to be in sync without much effort.
None of these fields should be reassigned once they are created, so
they are all readonly.

Except by ioSocket, all are of internal use, so keep them private.
off() without an event name just unregisters all event handlers via
EventEmitter.

The comment implied offAny() would do this, but it's not the case:
that just removes the onAny() handlers.

If we really wanted to remove all event handlers for everything we
should be calling:
 - offAny()
 - offAnyOutgoing()
 - off()

But that would not match the wrapped behavior, so let's just keep it
simple.
@barbieri
Copy link
Contributor Author

barbieri commented Jan 8, 2025

ping :-)

1 similar comment
@barbieri
Copy link
Contributor Author

ping :-)

@barbieri
Copy link
Contributor Author

barbieri commented Feb 6, 2025

hi @rodgc , not sure if you're out or busy, let me know if I can help to get this merged :-)

@rodgc rodgc merged commit 04f35ac into rodgc:master Feb 8, 2025
@barbieri barbieri deleted the fix-typing branch February 8, 2025 20:31
@barbieri
Copy link
Contributor Author

barbieri commented Feb 8, 2025

thanks!

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

Successfully merging this pull request may close these issues.

2 participants