Skip to content

Dictionary constructors do not contain parameters from the supertype's constructor #188

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

Closed
ditman opened this issue Feb 28, 2024 · 4 comments
Assignees

Comments

@ditman
Copy link
Member

ditman commented Feb 28, 2024

The FilePropertyBag constructor only accepts the lastModified property, however since it extends BlobPropertyBag, it should also support (and "pass through"1) type and endings (MDN)

Is there any way of expressing this in the generated code, or does this need a specific helper?

Footnotes

  1. This pass-through is probably a noop, there's no need to call the super constructor for this in JS, it's just that these PropertyBags should merge all the constructor arguments as you go down the inheritance chain?

@ditman ditman changed the title FilePropertyBag factory does not accept all possible options. FilePropertyBag factory does not accept all available options. Feb 28, 2024
@kevmoo
Copy link
Member

kevmoo commented Feb 28, 2024

CC @srujzs

@srujzs
Copy link
Contributor

srujzs commented Feb 28, 2024

Ah, it looks like we're not handling dictionary inheritance properly:

https://webidl.spec.whatwg.org/#dfn-dictionary

A given dictionary value of type D can have entries for each of the dictionary members defined on D and on any of D’s inherited dictionaries.

I'll modify the title to be more general. We likely want to prioritize this for the next release.

@srujzs srujzs changed the title FilePropertyBag factory does not accept all available options. Dictionary constructors do not contain parameters from the supertype's constructor Feb 28, 2024
@srujzs srujzs self-assigned this Feb 28, 2024
@ditman
Copy link
Member Author

ditman commented Feb 28, 2024

(I supposed this was a more general issue, but didn't check other dictionary constructors hehe)

@srujzs
Copy link
Contributor

srujzs commented Mar 4, 2024

Hmm, I'm thinking about this a bit more and not quite sure what the right answer is. An example where this might be a bad idea is DragEventInit. DragEventInit is a subtype of MouseEventInit, EventModifierInit, UIEventInit, and EventInit: https://github.com/w3c/webref/blob/ac13138cc839f523975fe4308495329d36a8a578/ed/idl/html.idl#L1767.

But DragEvent only cares about the one member DragEventInit exposes: https://developer.mozilla.org/en-US/docs/Web/API/DragEvent/DragEvent, so we end up exposing a constructor with tens of members that are all irrelevant. This may be okay as long as none of the supertypes' members are required, but it looks very confusing:

  external factory DragEventInit({
    bool bubbles,
    bool cancelable,
    bool composed,
    Window? view,
    int detail,
    int which,
    bool ctrlKey,
    bool shiftKey,
    bool altKey,
    bool metaKey,
    bool modifierAltGraph,
    bool modifierCapsLock,
    bool modifierFn,
    bool modifierFnLock,
    bool modifierHyper,
    bool modifierNumLock,
    bool modifierScrollLock,
    bool modifierSuper,
    bool modifierSymbol,
    bool modifierSymbolLock,
    int screenX,
    int screenY,
    int clientX,
    int clientY,
    int button,
    int buttons,
    EventTarget? relatedTarget,
    DataTransfer? dataTransfer,
  });

Maybe the documentation is incomplete and it should really say that it can inherit from the parent like with MouseEventInit: https://developer.mozilla.org/en-US/docs/Web/API/MouseEvent/MouseEvent#options.

srujzs added a commit that referenced this issue Mar 5, 2024
… empty object when there are no fields (#197)

Dictionaries can contain their supertypes' members as well and therefore, these constructors should accept them.

https://webidl.spec.whatwg.org/#idl-dictionaries
#188

We also emit some empty dictionary constructors, but because they are empty, they are treated as named constructors instead of object literal constructors. They should instead call JSObject() to create an empty object.

Since some supertype fields are marked required, this is technically a breaking change as calling the same dictionary constructor requires those members now. Therefore, bump to 0.6.0-wip.
@srujzs srujzs closed this as completed Mar 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

3 participants