-
Notifications
You must be signed in to change notification settings - Fork 2.9k
Add guidance on IDL attribute design #11153
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
base: main
Are you sure you want to change the base?
Add guidance on IDL attribute design #11153
Conversation
8702b17
to
5bf19d7
Compare
5bf19d7
to
1645d18
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for writing this up!
attribute handling and to help infer usage.</p> | ||
|
||
<p>The following provides some guidance on when each IDL attribute type is | ||
typically useful, with examples.</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think I would invert the advice. Given the types of content attributes we have, how would you go about best reflecting them. Because I think that's the question someone reading this is trying to answer, right?
I think we should also cover nuances such as "limited to known values", which we'd like to see used for new attributes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@annevk Got it, I've rewritten this introductory section to clarify that the content attribute informs IDL attribute reflection, and also included a note on encouraging "limited to only known values". I've also updated the title of this section to be "Designing attributes" if you think that's a better description (since it doesn't focus on just the IDL side).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This does seem like an improvement. But the majority of the advice is still centered around IDL types instead of content attribute types?
I would instead expect that we start with the content attribute types and then guide people towards the best IDL types. To sketch:
- If you have a boolean attribute, use
boolean
. - If you have an enumerated attribute, use
DOMString
, "limited to only known values", and in rare cases it might be appropriate to useDOMString?
. - If you have a string, use
DOMString
. - If you have an ID reference, use
Element
- If you have a list of ID references, use
FrozenList<Element>
Closes #11078
This PR provides guidance in the "Common DOM Interfaces" section on designing IDL attributes with appropriate types via a new "Designing IDL attributes" section. I also moved the following existing spec note on
FrozenArray<T>
naming to this new section as it fits better: