initialize a sensor object: Explicitly require |options| to be a SensorOptions

[rakuco]

This algorithm, invoked by the constructor of interfaces deriving from Sensor, was accepting any dictionary. This meant it was possible, for example, for any member (including frequency) to actually have any type whatsoever.

Be more strict and require options to be a SensorsOptions instance, or an instance of a dictionary inheriting from SensorOptions.

---

Preview | Diff

[rakuco]

cc @domenic: is this language enough to indicate that this algorithm accepts "a dictionary and dictionaries inheriting from it" or do I need to make it more explicit?

[kenchris]

I don't think we want to do that due to being future proof. In the future we add new members, it becomes a different version of the SensorOptions dict, so code written for a newer browser, might start failing in an older browser, which would be bad.

I think that we just need to look for the presence of those members and check their types, e.g. frequency should be a number and not an object, or a boolean etc.

[kenchris]

Look, this is what MutationObserver is doing here: https://dom.spec.whatwg.org/#dom-mutationobserver-observe

But I don't know how to handle frequency. Should we let a boolean true convert into 1? or actually do if Type(frequency) is not Number, throw TypeError? @domenic

[rakuco]

I don't think we want to do that due to being future proof. In the future we add new members, it becomes a different version of the SensorOptions dict, so code written for a newer browser, might start failing in an older browser, which would be bad.

Adding new members would be similar to defining a new dictionary inheriting from SensorOptions, which is what we already do with e.g. AccelerometerSensorOptions, GyroscopeSensorOptions etc. Even if you added new members to SensorOptions itself or change its existing member type(s), at this point you're already dealing with an IDL SensorOptions dictionary and the ES->IDL has already happened. The idea here is to just make it more clear that we want a SensorOptions dictionary, not any dictionary, so that we can count on the types of the members being what we want at the very least.

I think that we just need to look for the presence of those members and check their types, e.g. frequency should be a number and not an object, or a boolean etc.

This (as well as implicit type conversions) should have happened at the ES->IDL boundary (e.g. in Gyroscope's constructor) and shouldn't be a concern at this point as far as I can see.

Look, this is what MutationObserver is doing here: https://dom.spec.whatwg.org/#dom-mutationobserver-observe

Note how there are no type checks happening there though. The steps are just checking the values (and whether certain fields exist when the members are not required and don't have a default value).

But I don't know how to handle frequency. Should we let a boolean true convert into 1? or actually do if Type(frequency) is not Number, throw TypeError? @domenic

This is all done automatically at the ES->IDL boundary. Per https://webidl.spec.whatwg.org/#es-dictionary a JS true would be converted following https://webidl.spec.whatwg.org/#es-double which means it becomes 1 in this case.

[rakuco]

cc @domenic: is this language enough to indicate that this algorithm accepts "a dictionary and dictionaries inheriting from it" or do I need to make it more explicit?

I've asked about this in whatwg/webidl#1199 and it looks like this change is fine. @anssiko please feel free to merge at your earliest convenience.

[reillyeon]

I don't think we want to do that due to being future proof. In the future we add new members, it becomes a different version of the SensorOptions dict, so code written for a newer browser, might start failing in an older browser, which would be bad.

This is an issue whenever a new member is added to an existing IDL dictionary. Specification authors need to be careful to specify default values for such new members which don't change the original behavior.