-
Notifications
You must be signed in to change notification settings - Fork 107
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
Publish groups and snapshots #1060
Conversation
Discussed in today's WebDX call (notes). Philip: the bigger topic here is coming up with guidelines for groups. While groups are easy for like 50% of features, it gets harder for other features. Is grid animation part of animations, layout? Which group should structuredCloned be in? Patrick: MDN is very interested in groups, for information architecture purposes. We should revive this discussion with MDN and OWD. |
The key question blocking this for me is whether we should change the default export of the package, or just add named exports for groups and snapshots. In either case we can make the following work: import { features, groups } from 'web-features'; But what should the default export be? import thing from 'web-features'; What is Both are easy to do, but I'm not sure what's typical for JS modules. |
My preference (though not strongly held) would be to not have a default export at all. If we change what we're exporting, it'll break in a more transparent way for consumers. I'd much rather get a This also makes it less risky for us to embellish the package in the future. For instance, if we add some |
a1fa8f7
to
1c58c67
Compare
1c58c67
to
3e84661
Compare
@ddbeck I've removed the default export so we only have named exports now. The remaining thing I need feedback on is the schema. It's still split into defs.schema.json and features.schema.json, but features.schema.json is no longer a great name. I can't quite tell why we have two files. What would need to be done to have a single self-contained schema file that we could then rename? |
I tinkered and it seems like defs.schema.json already is standalone. I've pushed a commit to remove features.schema.json. |
"additionalProperties": { | ||
"$ref": "#/definitions/FeatureData" | ||
}, | ||
"description": "Feature identifiers and data", | ||
"type": "object" |
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 thing you'll lose here is the patternProperties
constraint from features.schema.json
:
"patternProperties": {
"^[a-z0-9-]*$": {
"$ref": "defs#/definitions/FeatureData"
}
}
That said, if I rename a file to use underscores, ajv
seems to not catch the break with the schema. So maybe this doesn't matter, or never has? 🤷
See #44 (comment) for background
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 tried to find a way to express this with TypeScript but failed. It may be possible with https://www.typescriptlang.org/docs/handbook/2/template-literal-types.html but it wouldn't be pretty, and I can't imagine it would get turned into a pretty RegEx in JSON schema.
Maybe #1317 instead?
This is in order to simplify the schema: web-platform-dx#1060 (comment)
This is in order to simplify the schema: #1060 (comment)
Groups and snapshots are published! Yay! |
You can see a very simple visualization of groups and features that have parent groups at https://web-platform-dx.github.io/web-features-explorer/groups/. |
That was quick, thank you @captainbrosset! This view immediately makes some odd things clear, like the group for Grid while Flexbox doesn't have one. We'll need guidelines for groups as well :) |
The Web Features repo changed the structure of the data.json in this PR: web-platform-dx/web-features#1060 Instead of being a map of features, it is now an object with three top level keys: - features - groups - snapshots This is a breaking change and has caused us to start ingesting features as if they had IDs: features, groups, snapshots This commit fixes that by updating the defs.schema.json to be the latest version (v0.10.0). This file comes from the web features repo. After auto-generating the code for that schema file, this commit adjusts the code to use it. We previously had v0.6.0 of the schema. And since then, some optional fields have become required fields. So you will see the removal of some nil checks (since now the field is not nullable). Change-Id: I9861421bc53ec8966eee80c33742d2a7e94d744c
The Web Features repo changed the structure of the data.json in this PR: web-platform-dx/web-features#1060 Instead of being a map of features, it is now an object with three top level keys: - features - groups - snapshots This is a breaking change and has caused us to start ingesting features as if they had IDs: features, groups, snapshots This commit fixes that by updating the defs.schema.json to be the latest version (v0.10.0). This file comes from the web features repo. After auto-generating the code for that schema file, this commit adjusts the code to use it. We previously had v0.6.0 of the schema. And since then, some optional fields have become required fields. So you will see the removal of some nil checks (since now the field is not nullable). Change-Id: I9861421bc53ec8966eee80c33742d2a7e94d744c
No description provided.