Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Parse browser compatibility metadata #144
Parse browser compatibility metadata #144
Changes from 6 commits
d2bdca4
3abcea5
a17642f
c7d69d4
064d01b
0165b03
34ca862
fa3b0b4
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
The MDN exposes compatibility info for dictionary interfaces with an
_option
suffix e.g. https://github.com/mdn/browser-compat-data/blob/cfa15e085ceb88cdee391ae09bd52ee3674d5ecc/api/MediaDevices.json#L264C10-L264C29. We could handle this here by having a separate "dictionaryOptions" field inBCDInterfaceStatus
or we can just do the lookup in the generation script.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.
Ah, interesting. This would be metadata about parameters for an interface's method?
It looks like these options exist off properties? So we'd want
BCDInterfaceStatus
to containBCDPropertyStatus
items, and for those items to have optionaloptions
(anything w/ an_option
suffix).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.
Just to help me think about the organization in the bcd.json file, there are ~16
foo_option
instances in there (all fields off of interface properties?).https://gist.github.com/devoncarew/81c987aa55637b6014c5b400a7edf022
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.
Funnily enough, that info is actually a separate thing altogether and I think they're suffixed with
_parameter
: https://github.com/mdn/browser-compat-data/blob/c2761cb84874ff5b9b0a230e7a557a5a43b9b939/api/EventTarget.json#L160C10-L160C27. :D Also see https://developer.mozilla.org/en-US/docs/Web/API/EventTarget/addEventListener#browser_compatibility.This is for what options can go in an options object that is passed to an API. https://developer.mozilla.org/en-US/docs/Web/API/MediaDevices/getDisplayMedia#browser_compatibility
There's also "_parameter_optional" apparently for optional parameters: https://github.com/mdn/browser-compat-data/blob/c2761cb84874ff5b9b0a230e7a557a5a43b9b939/api/HTMLTableRowElement.json#L378.
I think we may want to validate all the strings and check for
_
to make sure we're handling the various types of compatibility info. There's also some guidance here: https://github.com/mdn/browser-compat-data/blob/main/docs/data-guidelines/index.md#parameters-and-parameter-object-features, but it doesn't seem to catch every variant unfortunately. Realistically, there are probably not enough APIs that use all the variants for this to matter from our perspective, but something to watch out for.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.
OK, taking another pass through this, it looks like neither
_option
nor_parameter
show up in the first two levels of the data (the interfaces and their properties).Separately, I do see some types we're generating interop code for that aren't represented in the BCD data. I expect we'll need to adjust how we parse and use the data as we start to filter how much of the IDL we generate code for.
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.
Right, we can do this in another CL to grab more information about a specific member or constructor.
I suspect the majority of these cases to be because the types are new and are still being drafted, and therefore MDN doesn't have information on them. It'll be useful to validate that, but I suspect if the type is not present, we can safely omit the type.
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.