neon-bindings / rfcs

RFCs for changes to Neon
Apache License 2.0
14 stars 9 forks source link

RFC: EventHandler #25

Closed geovie closed 5 years ago

geovie commented 5 years ago

Provide a way for bindings to call back into JavaScript from threads other than the Node.JS main thread

Rendered

Preliminary implementation: https://github.com/neon-bindings/neon/pull/375

amilajack commented 5 years ago

@geovie sorry for the delay on this. We'll review this soon

sylvain101010 commented 5 years ago

Hi, I would love to see this completed, merged and released, can I help for anything?

kjvalencik commented 5 years ago

@z0mbie42 this is being discussed both in this RFC as well as in the slack workspace. You are welcome to join the conversation if you have thoughts! We don't currently have an expected completion date, but hopefully not long!

kjvalencik commented 5 years ago

@dherman This looks good to me. Do you want to make a call for final comment period or given the demand for this feature, should we skip that?

dherman commented 5 years ago

I like the simplification and reduction of boilerplate that you get from just being able to compute the arguments list instead of having to explicitly call the handler. In the future if we wanted (say, for performance) a lower-level method that lets you schedule arbitrary logic that take the this and func as explicit parameters and lets you decide whether to call them:

cb.schedule_with(move |cx, this, func| { ... })

we could add that backwards-compatibly. I don't think it's urgent, though, and I think we should accept the RFC as-is. We can always add more methods later.

Awesome work, and thanks for keeping at it!

dherman commented 5 years ago

I've labelled the RFC as being in final comment period! Thanks again @geovie for your work on this.

dherman commented 5 years ago

🎉🎉🎉🎉🎉 RFC #25 is merged! 🎉🎉🎉🎉🎉

dherman commented 4 years ago

RFC #25 is implemented and merged behind a feature flag in neon-bindings/neon#375!