Drop duplicate CSS property definitions when curating data #555
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This adds a new data curation step that drops duplicate CSS property definitions from CSS extracts whenever possible, in other words when we know which definition is the authoritative one. The code contains a list of known superseding relationships between specs to choose the right one. See discussion in #127 for details. CSS properties that get re-defined in a delta spec are ignored, meaning that a naive merge of all curated CSS extracts will still contain such duplicates. A typical solution to end up with a consistent merge would be to exclude delta specs from that merge. This is left to data consumers as some may choose to rather live on the bleeding edge. The curated data may still contain duplicate CSS property definitions, but these will get caught by tests. The new tests also check that CSS extracts contain a base CSS property definition for all CSS properties that get extended (through `newValues`).
dontcallmedom
approved these changes
Apr 9, 2022
|
Hmm, there is actually a bug in the code, because Two additional notes:
|
The `requireFromWorkingDirectory` function used `require`, which caches objects in memory. That is good for modules and functions. That is usually a bad idea for JSON objects that code often modifies in place. The new loadJSON reads and parses JSON files from storage to avoid conflicts.
This adds unicity of CSS property definitions and presence of base CSS property definitions to the list of guarantees that we make for CSS extracts.
|
I also completed the READMEs to mention the new guarantees. The whole thing does not introduce any backward incompatibility, but should we still consider that this should trigger a major bump of the CSS package given that it drops (duplicate) property definitions from a number of CSS extracts? |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This adds a new data curation step that drops duplicate CSS property definitions from CSS extracts whenever possible, in other words when we know which definition is the authoritative one. The code contains a list of known superseding relationships between specs to choose the right one. See discussion in #127 for details.
CSS properties that get re-defined in a delta spec are ignored, meaning that a naive merge of all curated CSS extracts will still contain such duplicates. A typical solution to end up with a consistent merge would be to exclude delta specs from that merge. This is left to data consumers as some may choose to rather live on the bleeding edge.
The curated data may still contain duplicate CSS property definitions, but these will get caught by tests. The new tests also check that CSS extracts contain a base CSS property definition for all CSS properties that get extended (through
newValues).