Conversation
|
Hi @rictic, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution! The agreement was validated by Microsoft and real humans are currently evaluating your PR. TTYL, MSBOT; |
| readonly scrollWidth: number; | ||
| readonly tagName: string; | ||
| innerHTML: string; | ||
| readonly assignedSlot: HTMLSlotElement | null; |
There was a problem hiding this comment.
The property is optional on the spec, we should make it optional here so that undefined can be accepted as well.
There was a problem hiding this comment.
And the same for other places too.
There was a problem hiding this comment.
I initially wrote it like that, but I when I was doing some double-checking it looked like WebIDL's ? suffix is different from typescript's ?. Take a look at this and let me know what you think:
- For turning an IDL specified type
T?into an ES value, this sections seems like it's saying that only values fromTandnullare allowed: https://heycam.github.io/webidl/#idl-nullable-type - For turning an ES value into the IDL type
T?thenundefinedandnullare treated the same: https://heycam.github.io/webidl/#es-nullable-type
So for places where the DOM accepts an IDL type T? from ES code then T | null | undefined looks like the right TS type, but for readonly properties it seems like T | null is more correct.
Apologies if this debate has already been litigated and the decision to treat nullable values as also allowing undefined has been made across the board.
I have made the non-required members of dictionaries, like EventInit#scoped optional.
There was a problem hiding this comment.
Thanks for the links. Yeah I agree it makes more sense, since the readonly types are from DOM itself.
There was a problem hiding this comment.
I do not see why a readonly propoerty can not be undefined. this is useful for users who want to feature dection. this should be:
readonly assignedSlot?: HTMLSlotElement | null;There was a problem hiding this comment.
As I read the spec, in a browser that implements this API, assignedSlot will never be undefined.
It's true that in an environment without the API it could be undefined, but this is true of most browser APIs.
|
I noticed a copy-paste mistake in my earlier patch. I accidentally spelled |
|
Thank you @rictic for contributing! |
|
the new properties should be marked optional. |
4 participants
Based on https://www.w3.org/TR/2016/WD-shadow-dom-20160830/