Skip to content

Commit

Permalink
Drop duplicate CSS property definitions when curating data
Browse files Browse the repository at this point in the history
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`).
  • Loading branch information
tidoust committed Apr 9, 2022
1 parent c554932 commit d2215a0
Show file tree
Hide file tree
Showing 3 changed files with 202 additions and 1 deletion.
33 changes: 33 additions & 0 deletions test/css/all.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
const assert = require('assert').strict;
const path = require('path');
const css = require('@webref/css');
const index = require('../../curated/index.json');
const { definitionSyntax } = require('css-tree');

const curatedFolder = path.join(__dirname, '..', '..', 'curated', 'css');
Expand Down Expand Up @@ -45,6 +46,8 @@ const tempIgnore = [
describe(`The curated view of CSS extracts`, () => {
before(async () => {
const all = await css.listAll({ folder: curatedFolder });
const baseProperties = {};
const extendedProperties = {};

for (const [shortname, data] of Object.entries(all)) {
describe(`The CSS extract for ${shortname} in the curated view`, () => {
Expand All @@ -54,11 +57,24 @@ describe(`The curated view of CSS extracts`, () => {
assert(data.spec.title);
});

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 (!baseProperties[name]) {
baseProperties[name] = [];
}
baseProperties[name].push({ spec: data.spec, dfn: desc });
}
else if ((type === 'extended property')) {
if (!extendedProperties[name]) {
extendedProperties[name] = [];
}
extendedProperties[name].push({ spec: data.spec, dfn: desc });
}
if (tempIgnore.some(c => c.shortname === shortname &&
c.prop === prop && c.name === name)) {
continue;
Expand All @@ -73,6 +89,23 @@ describe(`The curated view of CSS extracts`, () => {
}
});
}

describe(`Looking at CSS properties, the curated view`, () => {
for (const [name, dfns] of Object.entries(baseProperties)) {
it(`contains only one "${name}" property definition`, () => {
assert.strictEqual(dfns.length, 1,
`defined in ${dfns.map(d => d.spec.title).join(', ')} (${dfns.map(d => d.spec.url).join(', ')})`);
});
}
});

describe(`Looking at extended CSS properties, the curated view`, () => {
for (const [name, dfns] of Object.entries(extendedProperties)) {
it(`contains a base definition for the "${name}" property`, () => {
assert(baseProperties[name], 'no base definition found');
});
}
});
});

// Dummy test needed for "before" to run and register late tests
Expand Down
162 changes: 162 additions & 0 deletions tools/drop-css-property-duplicates.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,162 @@
/**
* Drop CSS property definition duplicates for situations where definition in
* one spec is known to override the same definition in another spec.
*
*
* node tools/drop-css-property-duplicates.js [folder]
*
* ... where:
* - [folder] is the name of the folder that contains the data to parse
* and update (default is "curated")
*/

const fs = require('fs').promises;
const path = require('path');
const util = require('util');
const execFile = util.promisify(require('child_process').execFile);
const requireFromWorkingDirectory = require('./utils').requireFromWorkingDirectory;
const expandCrawlResult = require('reffy').expandCrawlResult;


/**
* Known superseding relationships between CSS/SVG specs
*/
const supersededBy = {
// All CSS modules supersede CSS 2.x
'CSS': '*',

// See note in https://drafts.csswg.org/css-align/#placement
// "The property definitions here supersede those in [CSS-FLEXBOX-1]"
'css-flexbox': 'css-align',

// https://github.com/w3c/csswg-drafts/issues/6434#issuecomment-877447360
'css-logical': 'css-position',

// See https://github.com/w3c/csswg-drafts/issues/6435
// (string-set property defined twice)
'css-gcpm': 'css-content',

// See https://github.com/w3c/csswg-drafts/issues/6433
// (shape-inside should get dropped from css-round-display)
'css-round-display': 'css-shapes',

// See note in https://svgwg.org/specs/strokes/#sotd
// "In the future, this specification will supersede the SVG 2 Stroke
// definition, however at this time the SVG 2 Stroke definition must be
// considered the normative definition."
'svg-strokes': 'SVG',

// TODO: confirm that CSS modules supersede SVG 2
'SVG': [
'css-images', // image-rendering
'css-logical', // inline-size
'css-shapes', // shape-inside, shape-margin
'css-ui', // pointer-events
'fill-stroke' // all properties in fill-stroke
]
};


async function dropCSSPropertyDuplicates(folder) {
const rawIndex = requireFromWorkingDirectory(path.join(folder, 'index.json'));
const index = await expandCrawlResult(rawIndex, folder, ['css']);

const properties = {};
for (const spec of index.results) {
if (!spec.css?.properties) {
continue;
}
for (const [name, dfn] of Object.entries(spec.css.properties)) {
if (dfn.newValues) {
// Extension of a base definition, does not count as duplicate
continue;
}
if (!properties[name]) {
properties[name] = [];
}
properties[name].push(spec);
}
}

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;
});
}
if (properties[name].length > 1) {
console.error(`- ${name} defined in ${properties[name].map(spec => `[${spec.shortTitle}](${spec.url})`).join(', ')}`);
duplicates += 1;
}
}

function getBaseJSON(spec) {
return {
spec: {
title: spec.title,
url: spec.crawled
}
};
}

async function saveCss(spec) {
// There are no comments in JSON, so include the spec title+URL as the
// first property instead.
const css = Object.assign({
spec: {
title: spec.title,
url: spec.crawled
}
}, spec.css);
const json = JSON.stringify(css, null, 2) + '\n';
const pathname = path.join(folder, 'css', spec.series.shortname + '.json')
await fs.writeFile(pathname, json);
};

for (const spec of index.results) {
if (spec.needsSaving) {
await saveCss(spec);
}
}

if (duplicates) {
throw new Error('Duplicate CSS properties found, see log for details');
}
}


/**************************************************
Export methods for use as module
**************************************************/
module.exports.dropCSSPropertyDuplicates = dropCSSPropertyDuplicates;


/**************************************************
Code run if the code is run as a stand-alone module
**************************************************/
if (require.main === module) {
const folder = process.argv[2] ?? 'curated';

dropCSSPropertyDuplicates(folder).catch(e => {
console.error(e);
process.exit(1);
});
}
8 changes: 7 additions & 1 deletion tools/prepare-curated.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ const {
createFolderIfNeeded,
requireFromWorkingDirectory,
copyFolder } = require('./utils');
const { applyPatches } = require('./apply-patches.js');
const { applyPatches } = require('./apply-patches');
const { dropCSSPropertyDuplicates } = require('./drop-css-property-duplicates');


/**
Expand Down Expand Up @@ -101,6 +102,11 @@ async function prepareCurated(rawFolder, curatedFolder) {
await fs.writeFile(crawlIndexFile, JSON.stringify(crawlIndex, null, 2));
console.log('- crawl outcome adjusted');

console.log();
console.log('Drop duplicate CSS property definitions when possible');
await dropCSSPropertyDuplicates(curatedFolder);
console.log('- done');

console.log();
console.log('Re-generate the idlparsed folder');
const idlExtracts = {};
Expand Down

0 comments on commit d2215a0

Please sign in to comment.