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

PromiseRejectionEvent constructor does not define what its "reason" member is set to #5303

Open
bzbarsky opened this issue Feb 18, 2020 · 6 comments

Comments

@bzbarsky
Copy link
Contributor

The spec for DOM event initialization via constructor assumes that event init dictionaries never have members that are not present: that is, all members are either required or have default values. If an event init dictionary ever has a member that is not present, the corresponding event object will end up with the corresponding member uninitialized.

The reason member of PromiseRejectionEventInit has no default value and is not required, so it hits this case. In browsers, the member gets consistently initialized to undefined, but it's not clear to me whether it's just because their ES Value storage has a default constructor that initializes to that or something. I know Spidermonkey's does.

In addition to that, in Gecko optional any always has an explicit (once you get past the IDL parser stage) default value of undefined that currently cannot be expressed in IDL syntax. This is sort of similar to how dictionaries used to have an explicit-but-unexpressible default of "empty dictionary".

I don't actually see an obvious way to fix the HTML spec here without changed to either https://dom.spec.whatwg.org/#concept-event-constructor to handle this case somehow or IDL to allow specifying = undefined on optional any values, or explicitly saying that any values default to undefined or something.

Note also https://www.w3.org/Bugs/Public/show_bug.cgi?id=23602 which got "migrated" into the long list in whatwg/webidl#777 that never got any progress. :( This case with a dictionary is somewhat interesting because for dictionaries there is in fact an observable difference, when converting to a JS object, between "no property" and "property whose value is undefined", so may we do in fact want to allow optional-and-no-default-value any in dictionary members. But we don't want that for the event init case.

I just looked, and I don't see obvious dictionaries with any members that get converted to JS objects, but it's probably a matter of time until someone adds one, so we may want to solve the general problem too. Suggestions welcome. One option would be to make this behave like dictionaries: require explicit "optional any = undefined" in function args, but actually have different representations for "not present" and "set to undefined" in dictionaries... except that won't round-trip, of course, given how dictionary init works.

@Ms2ger @annevk @domenic

@domenic
Copy link
Member

domenic commented Feb 18, 2020

Interesting. My inclination is to fix this in https://dom.spec.whatwg.org/#concept-event-constructor somehow.

  • Changing the HTML spec to default to = null, while it matches CustomEvent (the other event with an any member), seems like unnecessary churn and semantically less good than undefined.

  • Making all optional any values in dictionaries default to undefined seems subpar. It breaks the general idea that, if something is not passed and has no default, then it counts as "not present" at the post-IDL-bindings layer.

  • Introducing an = undefined syntax into IDL for this makes spec authors now have to be aware of the difference between any x; and any x = undefined in their IDL. The answer is that in the former case myDictionary["x"] is not present when authors pass {}, and in the latter case myDictionary["x"] is the JS value undefined when authors pass {}. But I'd rather not have this choice in the spec-writing ecosystem. The best case we'd end up with is something like "Use = undefined for *EventInit dicts, for complicated IDL reasons. Otherwise don't use it."

Having DOM handle this at the event layer, and letting spec authors stay away from these details, seems nicer to me.

As for "somehow"... well, I'm not sure, but I'll note that we already have a preexisting problem in event constructors along similar lines, which is whatwg/dom#600. Maybe we can solve both at the same time.

@annevk
Copy link
Member

annevk commented Feb 19, 2020

Why can't "not present" mean that the value is undefined? Is there a reason dictionaries should be able to distinguish but method calls should not? (I guess that discussion needs to be had in the IDL repository, but since we started...)

@bzbarsky
Copy link
Contributor Author

Why can't "not present" mean that the value is undefined?

That's just not what https://heycam.github.io/webidl/#dictionary-to-es step 3.1.2 does, if you meant the conversion to an Object.

Is there a reason dictionaries should be able to distinguish but method calls should not?

Basically this issue of conversion-to-JS, mostly. The other reasons that dictionaries and method calls behave differently for the other thing which always has a default value in method calls (dictionaries, that is) doesn't apply here: that's there to allow an optional dictionary member whose value is a dictionary with a required member, but that complication does not apply to any.

@domenic
Copy link
Member

domenic commented Feb 19, 2020

Basically this issue of conversion-to-JS, mostly.

In particular, consider cases like

dictionary D {
  Node n;
}

If you pass {} as a D argument, then n is "not present". It's not undefined, because that would be pretty weird: you said in the dictionary definition that if it's present, its type should be Node.

From that perspective, treating {} as { reason: undefined } for PromiseRejectionEventInit is similar to treating {} as { n: someKindOfDefaultNode } for D. Both would be weird.

@annevk
Copy link
Member

annevk commented Feb 19, 2020

But how is it different from optional Node n? Or alternatively, why should passing undefined throw for dictionaries?

@bzbarsky
Copy link
Contributor Author

That's true, but the same happens for optional Node arg, right? Passing undefined means the arg is not present. On the other hand, having optional any arg treat undefined as "not present" is a little weird, since any explicitly allows undefined as a possible value, which is what differentiates it from Node. Fundamentally, the fact that undefined is both a "missing marker" and a valid value for any is what complicates things here: we end up having to guess at intent, and ES-to-dictionary conversions lose information no matter what, because they treat "no prop" and "prop with explicit undefined value" identically.

I really should have just filed an IDL issue for the larger pieces. :( The big question here is what to do for event init dictionaries.

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

No branches or pull requests

3 participants