Skip to content

Drop most CSS value spaces duplicates #556

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.

Sign up for GitHub

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

Merged
merged 1 commit into from
Apr 13, 2022
Merged

Drop most CSS value spaces duplicates #556

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Drop most CSS value spaces duplicates 
This applies the same logic as for CSS properties to drop duplicates from CSS
extracts. Some duplicates remain: `<fit-content()>`, `<length>`, `<path()>`,
`<percentage>`, `<rect()>` and `<url>`.

No easy way to get rid of them, as functions and terms seem to be reused on
purpose with similar though slightly different meaning each time. These
duplicates are hardcoded into tests.

No update to guarantees in README as it's not clear what interesting guarantees
we can really make for now. Value spaces remain too messy to be directly usable,
e.g. see #127 (comment)
  • Loading branch information
@tidoust
tidoust committed Apr 11, 2022
commit 50e49f2b30e0e1138f4e995e6704ca150c3d33cb
75 changes: 70 additions & 5 deletions test/css/all.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,54 @@ const tempIgnore = [
{ shortname: 'svg-strokes', prop: 'valuespaces', name: '<dasharray>' }
];


// Valuespaces that are defined more than once...
const duplicatedValuespaces = [
// Defined in CSS Grid Layout Module Level 2 and CSS Box Sizing Module Level 3
// https://drafts.csswg.org/css-grid-2/
// https://drafts.csswg.org/css-sizing-3/
'<fit-content()>',

// Defined in CSS Images Module Level 3, CSS Positioned Layout Module Level 3
// and CSS Text Module Level 3 (and CSS Values but in prose so not extracted)
// https://drafts.csswg.org/css-images-3/
// https://drafts.csswg.org/css-position/
// https://drafts.csswg.org/css-text-3/
'<length>',

// Defined in CSS Shapes Module Level 1 and Motion Path Module Level 1
// https://drafts.csswg.org/css-shapes/
// https://drafts.fxtf.org/motion-1/
'<path()>',

// Defined in CSS Positioned Layout Module Level 3,
// CSS Mobile Text Size Adjustment Module Level 1 and CSS Text Module Level 3
// (and CSS Values but in prose so not extracted)
// https://drafts.csswg.org/css-position/
// https://drafts.csswg.org/css-size-adjust-1/
// https://drafts.csswg.org/css-text-3/
'<percentage>',

// Defined in CSS Masking Module Level 1 and CSS Shapes Module Level 1
// https://drafts.fxtf.org/css-masking-1/
// https://drafts.csswg.org/css-shapes/
'<rect()>',

// Defined in CSS Masking Module Level 1, CSS Values and Units Module Level 4
// and Filter Effects Module Level 1
// https://drafts.fxtf.org/css-masking-1/
// https://drafts.csswg.org/css-values-4/
// https://drafts.fxtf.org/filter-effects-1/
'<url>'
];


describe(`The curated view of CSS extracts`, () => {
before(async () => {
const all = await css.listAll({ folder: curatedFolder });
const baseProperties = {};
const extendedProperties = {};
const valuespaces = {};

for (const [shortname, data] of Object.entries(all)) {
describe(`The CSS extract for ${shortname} in the curated view`, () => {
Expand All @@ -60,21 +103,27 @@ describe(`The curated view of CSS extracts`, () => {
const spec = index.results.find(s => s.nightly.url === data.spec.url);
for (const { type, prop, value } of cssValues) {
for (const [name, desc] of Object.entries(data[prop])) {
if (!desc[value]) {
continue;
}
if ((type === 'property') && (spec.seriesComposition !== 'delta')) {
if ((type === 'property') && (spec.seriesComposition !== 'delta') && !desc.newValues) {
if (!baseProperties[name]) {
baseProperties[name] = [];
}
baseProperties[name].push({ spec: data.spec, dfn: desc });
}
else if ((type === 'extended property')) {
else if ((type === 'extended property') && desc[value]) {
if (!extendedProperties[name]) {
extendedProperties[name] = [];
}
extendedProperties[name].push({ spec: data.spec, dfn: desc });
}
else if ((type === 'value space') && (spec.seriesComposition !== 'delta')) {
if (!valuespaces[name]) {
valuespaces[name] = [];
}
valuespaces[name].push({ spec: data.spec, dfn: desc });
}
if (!desc[value]) {
continue;
}
if (tempIgnore.some(c => c.shortname === shortname &&
c.prop === prop && c.name === name)) {
continue;
Expand Down Expand Up @@ -106,6 +155,22 @@ describe(`The curated view of CSS extracts`, () => {
});
}
});

describe(`Looking at CSS valuespaces, the curated view`, () => {
for (const [name, dfns] of Object.entries(valuespaces)) {
if (duplicatedValuespaces.includes(name)) {
it(`contains more than "${name}" valuespace definitions`, () => {
assert(dfns.length >= 2);
});
}
else {
it(`contains only one "${name}" valuespace definition`, () => {
assert.strictEqual(dfns.length, 1,
`defined in ${dfns.map(d => d.spec.title).join(', ')} (${dfns.map(d => d.spec.url).join(', ')})`);
});
}
}
});
});

// Dummy test needed for "before" to run and register late tests
Expand Down
63 changes: 43 additions & 20 deletions tools/drop-css-property-duplicates.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -78,36 +78,59 @@ async function dropCSSPropertyDuplicates(folder) {
}
}

const valuespaces = {};
for (const spec of index.results) {
if (!spec.css?.valuespaces) {
continue;
}
for (const [name, dfn] of Object.entries(spec.css.valuespaces)) {
if (!valuespaces[name]) {
valuespaces[name] = [];
}
valuespaces[name].push(spec);
}
}

function filterSuperseded(spec, specs, type, name) {
const shortname = spec.series.shortname;
if ((spec.seriesComposition === 'delta') &&
specs.find(s => s !== spec && s.series.shortname === shortname)) {
// Property name both defined in delta spec and in base full spec,
// let's ignore the duplication.
return false;
}

const superseding = [supersededBy[shortname]].flat();
if (superseding[0] === '*' ||
specs.find(s => superseding.includes(s.series.shortname))) {
// Property name defined in a spec that supersedes the current one,
// drop the property definition from the current spec
delete spec.css[type][name];
spec.needsSaving = true;
return false;
}
return true;
}

let duplicates = 0;
for (const [name, specs] of Object.entries(properties)) {
if (specs.length > 1) {
properties[name] = specs.filter(spec => {
const shortname = spec.series.shortname;
if ((spec.seriesComposition === 'delta') &&
specs.find(s => s !== spec && s.series.shortname === shortname)) {
// Property name both defined in delta spec and in base full spec,
// let's ignore the duplication.
return false;
}

const superseding = [supersededBy[shortname]].flat();
if (superseding[0] === '*' ||
specs.find(s => superseding.includes(s.series.shortname))) {
// Property name defined in a spec that supersedes the current one,
// drop the property definition from the current spec
delete spec.css.properties[name];
spec.needsSaving = true;
return false;
}
return true;
});
properties[name] = specs.filter(spec => filterSuperseded(spec, specs, 'properties', name));
}
if (properties[name].length > 1) {
console.error(`- ${name} defined in ${properties[name].map(spec => `[${spec.shortTitle}](${spec.url})`).join(', ')}`);
duplicates += 1;
}
}

// Same logic for valuespaces, but there will remain a few duplicates
// (e.g. "<rect()>" or "<path()>") and that's fine.
for (const [name, specs] of Object.entries(valuespaces)) {
if (specs.length > 1) {
specs.filter(spec => filterSuperseded(spec, specs, 'valuespaces', name));
}
}

function getBaseJSON(spec) {
return {
spec: {
Expand Down