regl-scatterplot icon indicating copy to clipboard operation
regl-scatterplot copied to clipboard

fix: fixed pubsub related types

Open funmaker opened this issue 1 year ago • 1 comments

Write one to two sentences summarizing this PR

Current pubsub types do not match actual types used by pub-sub-es. This PR fixes them.

What was changed in this pull request?

Added missing return value in subscribe and added missing argument/overload for unsubscribe. Also added helper types for subscription object and handler.

Why is it necessary?

Because you can't use unsubscribe properly from events right now.

Checklist

  • [x] Provided a concise title as a semantic commit message (e.g. "fix: correctly handle undefined properties")
  • [ ] CHANGELOG.md updated
  • [ ] Tests added or updated
  • [ ] Documentation in README.md added or updated
  • [ ] Example(s) added or updated
  • [ ] Screenshot, gif, or video attached for visual changes

funmaker avatar May 09 '24 14:05 funmaker

Wow thanks for putting this together! I'm happy to merge this. One thing I wonder though is if we can actually remove the pub-sub typing here altogether as I recently added types to the pub-sub-es library. I know that @manzt added the pub-sub types here before pub-sub-es was typed but I think it might be better to fix any type issues in the pub-sub library instead of here.

I know this means more work but would you be willing to see if removing the pub-sub types here fixes the issue? I'm also happy to merge this fix for now and properly refactor things later.

flekschas avatar May 10 '24 00:05 flekschas

I'm closing this PR as I've refactored pub-sub-es to TypeScript to fix all the typing issues once and for all. Please let me know if you still run into issues with v1.9.0.

flekschas avatar Jun 01 '24 16:06 flekschas