Skip to content
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

Consider aligning the scripting API (webIDL) with the Interaction vocabulary #127

Closed
mjkoster opened this issue Jun 25, 2018 · 7 comments
Closed

Comments

@mjkoster
Copy link

mjkoster commented Jun 25, 2018

For Properties, consider property.read(), property.write() instead of property.get() and property.set()
Also consider property.observe() which returns Observable.

For Actions, consider action.invoke() instead of action.run()

For Events, consider event.subscribe() instead of reusing observeProperty semantics. It can return Observable.

@zolkis
Copy link
Contributor

zolkis commented Jul 2, 2018

Name changes to read, write, invoke are fine.

property.observe() returning an Observable adds one more indirection but it is fine. The problem is there is no unobserve(), or would we also define unobserve() that would map to the returned subscription.unsubscribe()? It becomes a bit messy.

About events, in the TD doc it already reuses property semantics.
We should then also discuss which Event model (Node, DOM) we support (or define our own).
Currently emitEvent() maps to whatever the underlying platform (JS runtime) supports.

Or do you just suggest that Event implements Observable, whereas Property should not (but instead have an observe() method)?

zolkis added a commit to zolkis/wot-scripting-api that referenced this issue Jul 2, 2018
Signed-off-by: Zoltan Kis <zoltan.kis@intel.com>
@mjkoster
Copy link
Author

mjkoster commented Jul 2, 2018

Maybe returning an observable isn't needed if the object implements observable. I'm not sure I said what I meant. For Events, we have a subscribe method rather than observe.

Maybe Property has an observe method in addition to read and write, and Event has a subscribe method. That's what I think I meant...

So we use the same method names in the Scripting API as the "rel" values we expose in TD

@zolkis
Copy link
Contributor

zolkis commented Jul 2, 2018

Currently both Property and Event extend Interaction which implements Observable, so both Event and Property have a subscribe() method. I thought you wanted to use the work observe() instead of subscribe() for Property for alignment. It is possible, and it could well return an Observable. The problem in that case is that despite of having observe().subscribe() the unsubscribing will anyway use the work unsubscribe() because of Observable. Defining an explicit unobserve() is messy, so the current solution seems to be the cleanest, even though the wording is subscribe and unsubscribe.

Actually if that is the issue, since we are using our own definition of Observable, we could change the names there: use Observable.observe() that returns a Subscription, where one could say subscription.cancel() or subscription.unobserve(). To avoid confusion with the ECMAScript proposal, we could even rename Observable to something else.

All we need to know how seriously we want to use a given vocabulary, e.g. observe/unobserve vs subscribe/unsubscribe.

We seem to agree that Events should anyway use subscribe/unsubscribe.

@mkovatsc
Copy link
Contributor

For Properties, consider property.read(), property.write() instead of property.get() and property.set()

@mjkoster We ended up with "read" and "write" in the Bindings TF while focusing on the protocol aspects. Yet properties traditionally use "get" and "set" to perform these operations (see also https://en.wikipedia.org/wiki/Property_(programming)).

Since WoT uses exactly this programming abstraction, it would be much cleaner to align in the other direction, that is, fix the terms in Interaction vocabulary -- which I expected to happen for the Korea target.

@mkovatsc
Copy link
Contributor

For Actions, consider action.invoke() instead of action.run()

Here I fully agree. I thought I documented this in another issue, but cannot find it at the moment. Anyhow, we should fix this for the upcoming release.

@mkovatsc
Copy link
Contributor

mkovatsc commented Aug 6, 2018

For now, it was most important to align between TD (i.e., forms vocabular, see w3c/wot-thing-description#179) and Scripting API.

For the following reasons, we decided in the call on 6 Aug 2018 to go for read() and write():

  • get/set usually for synchronous calls
  • read/write often used for async calls; we return a promise
  • get/set might be in conflict with getters and setters of objects
  • we want to allow for easy extension/decoration of the Scripting API to impement over API flavors

I will open an issue for node-wot and do the changes there before the 0.5.0 release.

@mkovatsc
Copy link
Contributor

mkovatsc commented Aug 6, 2018

We keep this issue open to see if we can find more and/or better reasons to decide this naming issue.

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

No branches or pull requests

3 participants