Skip to content

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

Open
wants to merge 9 commits into
base: main
Choose a base branch
from

Conversation

rahimabdi
Copy link
Contributor

@rahimabdi rahimabdi commented Mar 23, 2025

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:

Reflected IDL attributes of this type are strongly encouraged to have their identifier end in "Element" for consistency.

@rahimabdi rahimabdi force-pushed the rahim/add-attribute-design-guidance branch from 8702b17 to 5bf19d7 Compare March 23, 2025 16:22
Copy link
Member

@annevk annevk left a 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>
Copy link
Member

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.

Copy link
Contributor Author

@rahimabdi rahimabdi Mar 29, 2025

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).

Copy link
Member

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 use DOMString?.
  • 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>

@rahimabdi rahimabdi requested a review from annevk March 29, 2025 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Additional usage guidance for IDL attribute types (such as DOMString?)
2 participants