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

Why was namespacing dropped? #127

Closed
rwaldron opened this issue Sep 13, 2016 · 12 comments
Closed

Why was namespacing dropped? #127

rwaldron opened this issue Sep 13, 2016 · 12 comments

Comments

@rwaldron
Copy link
Contributor

rwaldron commented Sep 13, 2016

This appears across all of the new sensor APIs, so while I limit comments to this repo, any resolution will apply to all relevant specs.

The Generic Sensor spec describes a naming pattern that's counter to its original goals.

I've been back through the generic sensor spec's issue history and cannot find an exact point in time that we diverged from one of the important points of the original proposal: All newly specified sensor classes should be defined under a single global identifier "namespace". This was specifically called out as part of the problem that existing sensor specs designs created: they were piling identifiers onto the global/window object.

(For that exact case ^^^ these new spec are actually worse, since you can only see one at a time)

Where the following illustrates a superior view showing a set of related classes:


During all of the early conversations, in all of the example code and use cases, we made sure to write such things as:

let accelerometer = new Sensors.Accelerometer(); 
// or sometimes as: 
let accelerometer = new sensors.Accelerometer(); 
// ...either is better. 

Then this all changed. Unfortunately, I didn't have a chance to review that specific change at the time (admittedly, there was a period last year where I was swamped day-to-day and was unable review every change). [A look at the issues filed for that time period show that there wasn't discussion about the change)[https://github.com/w3c/sensors/issues?page=3&q=&utf8=%E2%9C%93] . If I had seen the change, I promise that I would've argued against it and would've absolutely died on that hill.

Here are some of the arguments I would've made and stand by to-the-day:

  1. Only a single Sensors or sensors global is created

  2. When defined as property of an object that's appropriately named, ie. Sensors or sensors, it's clear to all readers of that source code that all things defined there are going to be a sensor, or relevant to sensors—so their own names don't have to explicitly include the word "Sensor". This point may be seen as partially subjective, but I think that most would agree that when naming an accelerometer (or whatever) class, that Accelerometer is better than AccelerometerSensor. (There is presently 4 years of empirical evidence supporting the last statement).

  3. A list of available sensors can be easily produced programmatically:

    > Object.keys(sensors);
    [
      "Accelerometer",
      "Gyroscope",
      "Magnetometer"
    ]
  4. Class names can be "shorthand" aliased via destructuring:

    let { Accelerometer, Gyroscope } = Sensors;
    let a = new Accelerometer();
    let g = new Gyroscope();

Proposal

  • The Generic Sensor specification should define a single global object, named Sensors or sensors.
  • The Generic Sensor specification should mandate that all sensor classes are defined as the value of a property, whose name is the sensor of which is represented (eg. Accelerometer), on the single global object, named Sensors or sensors.
@anssiko
Copy link
Member

anssiko commented Sep 13, 2016

Thanks for the detailed issue. We'll have F2F next Monday and Tuesday, and we'll surely take this issue to the agenda: https://www.w3.org/2009/dap/wiki/LisbonF2F2016#Agenda

You're welcome to call in, although the timezone might not be the perfect fit. We could try to arrange the slots slightly to better accommodate remote participants if needed.

@anssiko anssiko closed this as completed Sep 13, 2016
@anssiko anssiko reopened this Sep 13, 2016
@anssiko
Copy link
Member

anssiko commented Sep 13, 2016

(When I thumb type, I close issues accidentally. And introduce typos. Doh.)

@rwaldron
Copy link
Contributor Author

You're welcome to call in, although the timezone might not be the perfect fit. We could try to arrange the slots slightly to better accommodate remote participants if needed.

I don't want to disrupt the schedule so close to the actual meeting—but I would like to address this issue. How flexible is the present time, or even: how can I be the least inconvenient and the most respectful to other members' time?

(When I thumb type, I close issues accidentally. And introduce typos. Doh.)

Same here ;)

@anssiko
Copy link
Member

anssiko commented Sep 14, 2016

We could take this issue as the last one on Monday 19 Sep around 12:30 UTC+00:00. Would that work for you?

We'll include call in details to the wiki ahead the meeting.

@marcoscaceres
Copy link
Member

WebIDL now supports namespaces, so you could get a real "sensors" namespace... however, I'm not sure why WebIDL namespaces are currently limited to operations (and don't seem to allow attributes, which would serve as the constructors)? @domenic, is something funky about the API design here?

As currently proposed (using Sensors.whatevs) in the OP, the constructors would be statics on the Sensor interface, which is not really what we want IMO. I think we want @rwaldron's design of, for example:

const acc = new sensors.Accelerometer();

@domenic
Copy link

domenic commented Sep 14, 2016

In general we didn't see any advantage in sprinkling dots everywhere. I.e. XSensor is better (fits with existing practice, etc.) vs. sensors.X.

@riju
Copy link
Contributor

riju commented Sep 15, 2016

@rwaldron : Regarding point 3. A list of available sensors can be easily produced programmatically ..

Presence of say AmbientLightSensor in Object.keys(sensors), does not mean the system has a AmbientLightSensor, it means the browser simply has bindings for AmbientLight sensor. We need to make a sync call like start() to find out if the underlying platform actually has a sensor.

Also with Android API level 24, there is a concept of dynamic sensor callback which is used for receiving notifications from the SensorManager when dynamic sensors are connected or disconnected.

Do you want proposed global object sensor object to hold only available sensors for that platform ?
Does that mean we need to update the sensor object every time a sensor's connection state changes ?

I understand the "grouping related classes under a namespace" reasoning, but cannot see benefits beyond that.

@tobie
Copy link
Member

tobie commented Sep 19, 2016

@rwaldron:

The main problem here, outside of preferences expressed by different folks on this thread, is that there's no way to express this in WebIDL. The newly added namespace (arguably a misnomer) doesn't let us do that either (it can't contain constructors).

As a side note, there is no plan to call Accelerometer, AccelerometerSensor. It should just be Accelerometer (see Extensibility section on naming).

While I hear your concerns about the lack of namespacing, I do also have some concerns about namespacing:

  1. it's inconsistent with the rest of the platform,
  2. it might delay shipping the spec and implementation for years (or at least until we get ES6 modules in the browser),
  3. the benefits are essentially cosmetic.

I'd be much more inclined to have a conversation around this if we had a clearer path as to what it would imply time-wise, in relationship to ES6 modules, and in WebIDL terms to namespace concrete sensors. It would be good to hear back from implementors on this topic and it would be super useful to know if there were other parts of the platform being worked on right now that would adopt a similar design.

@rwaldron
Copy link
Contributor Author

@tobie according to @marcoscaceres, namespaces are now supported.


@domenic

In general we didn't see any advantage in sprinkling dots everywhere. I.e. XSensor is better (fits with existing practice, etc.) vs. sensors.X.

This isn't a compelling (or even substantive) argument in a runtime that supports destructuring:

const { Accelerometer, Gyroscope } = sensors;
// no more "sprinkled" dots. 

@riju

I understand the "grouping related classes under a namespace" reasoning, but cannot see benefits beyond that.

That is the point I consider to be the most important.


Sorry I was unable to attend any meeting or call, I had a doctors appointment at 9:30 (UTC-04:00, presently) which was an hour away by car.

@tobie
Copy link
Member

tobie commented Sep 19, 2016

@rwaldron:

@tobie according to @marcoscaceres, namespaces are now supported.

See my comment above:

"The newly added namespace (arguably a misnomer) doesn't let us do that either (it can't contain constructors)."

See also whatwg/webidl#177.

@rwaldron
Copy link
Contributor Author

Oh wow. Thanks for following up expeditiously

@tobie tobie changed the title LaughometerSensor vs Laughometer? Why was namespacing dropped? Sep 20, 2016
@tobie
Copy link
Member

tobie commented Sep 20, 2016

Proposed resolution: we can't namespace for now. We'll reconsider when it's more practical to do so.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants