search

mondaycom / monday-sdk-js

Node.js and JavaScript SDK for developing over the monday.com platform
https://monday.com
MIT License
87 stars 39 forks source link

Possibly rewrite MondayClientSdk#listen to have an accompanying #unlisten #138

Closed grahhnt closed 8 months ago

grahhnt commented 9 months ago

Related #137

Instead of returning a function to unsubscribe/unlisten from #listen, supply a method to unlisten based on event name or function reference

See also socket.io's event management but not just limited to socket.io

gregra81 commented 8 months ago

@grahhnt can you please explain the goal of this request? What problem are you encountering?

grahhnt commented 8 months ago

@gregra81 i guess this is more of personal preference as this is the first library I've used that gave the "undo" action as a returned function

This was also sparked due to the type definitions not matching actual implementation (see the related PR)

gregra81 commented 8 months ago

@grahhnt Got it. This is indeed a preference that was chosen years ago when it was implemented. BTW, as a side note, react's useEffect is also taking this approach, its return function is intended for clean up.

About the types PR - we'll take this and review. In the meantime I'm closing this one