new constructors for WebIDL bindings introduce undefined behavior by not setting default values

[pablosichert]

Describe the Bug

The definitions for WebIDL dictionaries contain information if fields are optional or required by prefixing them with required or marking the type ?. However, there's a third option: fields can be neither required nor ?, but have a default value after =.

Currently, code generation for constructor bindings does not set default values in the body of the new function. Therefore, creating a dictionary with new() and immediately reading a field that should have a default value is undefined behavior.

Steps to Reproduce

E.g. for the first dictionary declaration I could find going alphabetically in the enabled WebIDLs that uses default values:

wasm-bindgen/crates/web-sys/webidls/enabled/AnalyserNode.webidl

Lines 13 to 18 in e78db23

	dictionary AnalyserOptions : AudioNodeOptions {
	unsigned long fftSize = 2048;
	double maxDecibels = -30;
	double minDecibels = -100;
	double smoothingTimeConstant = 0.8;
	};

use web_sys::{AnalyserOptions, AnalyserOptionsGetters as _};
let analyzer_options = AnalyserOptions::new();
let fft_size = analyzer_options.fft_size(); // Uncaught Error: expected a number argument, found undefined
println!("{}", fft_size);

This example is using the getters as implemented in #3933, but the same case can be constructed by many Web APIs that take a dictionary with properties containing default values as argument.

Expected Behavior

2048

Actual Behavior

imported JS function that was not marked as `catch` threw an error: expected a number argument, found undefined

Additional Context

The faulty generated code for AnalyserOptions::new that does not set default values:

wasm-bindgen/crates/web-sys/src/features/gen_AnalyserOptions.rs

Lines 30 to 38 in e78db23

	impl AnalyserOptions {
	#[doc = "Construct a new `AnalyserOptions`."]
	#[doc = ""]
	#[doc = "*This API requires the following crate features to be activated: `AnalyserOptions`*"]
	pub fn new() -> Self {
	#[allow(unused_mut)]
	let mut ret: Self = ::wasm_bindgen::JsCast::unchecked_into(::js_sys::Object::new());
	ret
	}

I discovered this by working on WebIDL dictionary getters in #3933 and thinking about the implications of unsetting dictionary values in #3921 (comment).

Not allowing to safely read properties that are known to exist on a dictionary would require using Option in return values of property getters and require to be unnecessarily defensive when reading them.

[MOZGIII]

It is not really an undefined behavior - they just create an empty object with no fields assigned. This is valid, and the defaults are applied at the receiver end.

[pablosichert]

@MOZGIII I don't think that's the spirit of the WebIDL dictionary definitions. In that case having the concept of a ? marker would be superfluous since every field is optional.

Also, WebIDL APIs that return dictionaries always have the properties set that are not marked with ?.

Initializing these fields properly would make the consumer side much more streamlined.

If, for some reason, you are not interested in initializing the fields fully, you could still manually call JsCast::unchecked_into<MyDictionary>(Object::new()), but that should not make the consumer side unneccesarily convoluted.

[daxpedda]

I'm definitely not an expert in JS/Web APIs, but I believe there are some misconceptions here.

WebIDL dictionary and enum types are not translated to JS by web browsers. So when using these types in JS in the browser they are plain objects (not entirely sure about the terminology) and therefor don't contain default values.

In comparison, WebIDL interface types do get a corresponding JS class which can be instantiated.

While I do understand where this is coming from, unfortunately web-sys is not the place to do this. Its supposed to expose unadulterated and unopinionated bindings to the Web API. Of course there is a need for a more user-friendly API, but clearly web-sys is too low-level to do this. Just looking at the amount of error handling each call requires, it is terribly cumbersome to use, but so are all bindings. I believe gloo was made to build on top of web-sys to provide exactly this kind of practical and user-friendly API that is desired.

I might even dare to say that web-sys shouldn't have implemented dictionaries in the first place, but now after #3898 its actually justified for performance reasons.

On a side note, as there is often no way to actually query the defaults, we can't really provide them. While it is true that we could follow the spec here, not all browsers follow the spec and it doesn't actually expose if the default was set or not, which does sometimes make a difference, according to the spec.

And yes, #3933 has to return Option because it might not be set.

---

I'm going to go ahead and close this, but please continue the discussed here if there is more left unsaid or if I was wrong about something.