search

WICG / input-for-workers

Specification for exposing input events to workers
Other
25 stars 6 forks source link

How many targets can be added to the Worker? #9

Closed Kaiido closed 4 years ago

Kaiido commented 4 years ago

From the current API design, it's not quite clear if it is expected that a single event target will be added to the Worker or if multiple will.

In one hand, the API uses an eventtargetadded event to notify the scripts when a new target is added, this sounds like it should be possible for multiple such events to fire, for instance if the main script calls multiple times worker.addEventTarget().
But in the mean time, there doesn't seem to be anything allowing to recognize what target has been added.

So if we have for instance 

 worker.addEventTarget( document.getElementById('canvas1') );
 worker.addEventTarget( document.getElementById('canvas2') );

Then in Worker, it's currently unclear how we can know if the eventtargetadded.target is canvas1 or canvas2.

Not sure what would be the best move here.
Certainly as an author, I can see cases where I want my scripts to be able to handle several targets, but also I see how this makes the API a lot more complex.

NavidZ commented 4 years ago

@mustaqahmed FYI

Thanks for filing this issue. You are right. This was brought up early on in the development cycle and has been addressed with the context attribute in EventDelegateOptions.

That way you can pass along whatever identifier or others things along with the target you are passing to the worker thread. On the worker side the target reference to multiple target will be different and you can have different listeners or just store those contexts for them on the worker side when getting eventtargetadded event and do as you wish based on those context attributes.

Ltw, the latest form of the API is not like what you have and what is in the spec right now. There is another proposal in flight in this PR that makes it more like creating a mirror object from a target on the main thread side and then you can pass it along to any thread. But your concern still exists in that solution as well and context field needs to be used by the developer to address that.

Kaiido commented 4 years ago

Thank you for the prompt response. So that's what context was for. Now I get it ;-).

The new version seems a lot better to me.
At least it's now clear that the authors should handle everything, as part of the postMessage data, just like with any other transferables.
Not sure there should even be any context there. I think the simpler the clearer.

I have some comments on that PR though, but I'll leave them there directly and close this issue.