Simplify and document the dist generation flow (#1233)

Parts of this were more complicated than they need to be because initially the dist file was a modified version of the source with comments preserved. Now that it is just computed data, simplifications are possible. Also, only call computeBaseline once with the compat keys that will actually be used. This does mean that fixing one warning could reveal another instead of showing both at the same time, but that's not so awful.
web-platform-dx · Jun 12, 2024 · 0eff225 · 0eff225
1 parent fe05aae
commit 0eff225
Show file tree

Hide file tree

Showing 5 changed files with 54 additions and 108 deletions.
diff --git a/features/anchor-positioning.yml.dist b/features/anchor-positioning.yml.dist
@@ -91,7 +91,6 @@ compat_features:
   - css.properties.bottom.anchor
   - css.properties.height.anchor-size
   - css.properties.inline-size.anchor-size
-  - css.properties.inset.anchor
   - css.properties.inset-area
   - css.properties.inset-area.block-end
   - css.properties.inset-area.block-start
@@ -128,12 +127,13 @@ compat_features:
   - css.properties.inset-area.y-self-end
   - css.properties.inset-area.y-self-start
   - css.properties.inset-area.y-start
-  - css.properties.inset-block.anchor
   - css.properties.inset-block-end.anchor
   - css.properties.inset-block-start.anchor
-  - css.properties.inset-inline.anchor
+  - css.properties.inset-block.anchor
   - css.properties.inset-inline-end.anchor
   - css.properties.inset-inline-start.anchor
+  - css.properties.inset-inline.anchor
+  - css.properties.inset.anchor
   - css.properties.justify-items.anchor-center
   - css.properties.justify-self.anchor-center
   - css.properties.left.anchor

diff --git a/features/border-image.yml.dist b/features/border-image.yml.dist
@@ -15,13 +15,13 @@ status:
     safari_ios: "9.3"
 compat_features:
   - css.properties.border-image
-  - css.properties.border-image.fill
-  - css.properties.border-image.gradient
-  - css.properties.border-image.optional_border_image_slice
   - css.properties.border-image-outset
   - css.properties.border-image-repeat
   - css.properties.border-image-repeat.round
   - css.properties.border-image-repeat.space
   - css.properties.border-image-slice
   - css.properties.border-image-source
   - css.properties.border-image-width
+  - css.properties.border-image.fill
+  - css.properties.border-image.gradient
+  - css.properties.border-image.optional_border_image_slice
diff --git a/features/overflow-shorthand.yml.dist b/features/overflow-shorthand.yml.dist
@@ -14,11 +14,11 @@ status:
     safari_ios: "16"
 compat_features:
   - css.properties.overflow
-  - css.properties.overflow.clip
-  - css.properties.overflow.multiple_keywords
   - css.properties.overflow-x
   - css.properties.overflow-x.clip
   - css.properties.overflow-y
   - css.properties.overflow-y.clip
+  - css.properties.overflow.clip
+  - css.properties.overflow.multiple_keywords
   - css.types.overflow
   - css.types.overflow.clip
diff --git a/features/transforms3d.yml.dist b/features/transforms3d.yml.dist
@@ -15,15 +15,15 @@ status:
 compat_features:
   - css.properties.backface-visibility
   - css.properties.perspective
-  - css.properties.perspective.none
   - css.properties.perspective-origin
   - css.properties.perspective-origin.bottom
   - css.properties.perspective-origin.center
   - css.properties.perspective-origin.left
   - css.properties.perspective-origin.right
   - css.properties.perspective-origin.top
-  - css.properties.transform.3d
+  - css.properties.perspective.none
   - css.properties.transform-style
+  - css.properties.transform.3d
   - css.types.transform-function.matrix3d
   - css.types.transform-function.perspective
   - css.types.transform-function.rotate3d

diff --git a/scripts/dist.ts b/scripts/dist.ts
@@ -74,83 +74,66 @@ function checkDistFile(sourcePath: string, distPath: string): boolean {
  * successful, generates a `status` block.
  */
 function toDist(sourcePath: string): YAML.Document {
-  const yaml = YAML.parseDocument(
-    fs.readFileSync(sourcePath, { encoding: "utf-8" }),
-  );
+  const source = YAML.parse(fs.readFileSync(sourcePath, { encoding: "utf-8" }));
   const { name: id } = path.parse(sourcePath);
 
-  const taggedCompatFeatures = (
-    tagsToFeatures.get(`web-features:${id}`) ?? []
-  ).map((f) => `${f.id}`);
-
-  const overridden = {
-    compatFeatures: yaml.toJS().compat_features,
-    status: yaml.toJS().status,
-  };
-
-  const generated = {
-    compatFeatures: taggedCompatFeatures.length
-      ? taggedCompatFeatures
-      : undefined,
-    status: taggedCompatFeatures.length
-      ? computeBaseline({
-          compatKeys: taggedCompatFeatures as [string, ...string[]],
-          checkAncestors: false,
-        })
-      : undefined,
-    statusByCompatFeaturesOverride: Array.isArray(overridden.compatFeatures)
-      ? computeBaseline({
-          compatKeys: overridden.compatFeatures as [string, ...string[]],
-          checkAncestors: false,
-        })
-      : undefined,
-  };
+  // Collect tagged compat features. A `compat_features` list in the source
+  // takes precedence, but can be removed if it matches the tagged features.
+  const taggedCompatFeatures = (tagsToFeatures.get(`web-features:${id}`) ?? [])
+    .map((f) => `${f.id}`)
+    .sort();
 
-  warnOnNeedlessOverrides(id, overridden, generated);
+  if (source.compat_features) {
+    source.compat_features.sort();
+    if (isDeepStrictEqual(source.compat_features, taggedCompatFeatures)) {
+      logger.warn(
+        `${id}: compat_features override matches tags in @mdn/browser-compat-data. Consider deleting this override.`,
+      );
+    }
+  }
 
-  const dist = new Document({});
-  insertHeaderComments(dist, id);
+  // Compute the status. A `status` block in the source takes precedence, but
+  // can be removed if it matches the computed status.
+  let computedStatus = computeBaseline({
+    compatKeys: source.compat_features ?? taggedCompatFeatures,
+    checkAncestors: false,
+  });
 
-  if (!overridden.compatFeatures && generated.compatFeatures) {
-    insertCompatFeatures(dist, generated.compatFeatures);
+  if (computedStatus.discouraged) {
+    logger.warn(
+      `${id}: contains at least one deprecated compat feature and can never be Baseline. Was this intentional?`,
+    );
   }
 
-  if (!overridden.status) {
-    const status = generated.statusByCompatFeaturesOverride ?? generated.status;
-    if (status) {
-      if (status.discouraged) {
-        logger.warn(
-          `${id}: contains at least one deprecated compat feature and can never be Baseline. Was this intentional?`,
-        );
-      }
-      insertStatus(dist, JSON.parse(status.toJSON()));
+  computedStatus = JSON.parse(computedStatus.toJSON());
+
+  if (source.status) {
+    if (isDeepStrictEqual(source.status, computedStatus)) {
+      logger.warn(
+        `${id}: status override matches computed status. Consider deleting this override.`,
+      );
     }
   }
 
-  return dist;
-}
-
-function insertCompatFeatures(yaml: Document, compatFeatures: string[]) {
-  yaml.set("compat_features", compatFeatures);
-}
-
-function insertStatus(yaml: Document, status) {
-  // Create the status node and insert it just before "compat_features"
-  const statusNode = yaml.createPair("status", status);
-  assert(YAML.isMap(yaml.contents));
-  const targetIndex = yaml.contents.items.findIndex(
-    (item) => item.key.toString() === "compat_features",
-  );
-  yaml.contents.items.splice(targetIndex, 0, statusNode as YAML.Pair<any, any>);
-}
+  // Assemble and return the dist YAML.
+  const dist = new Document({});
 
-function insertHeaderComments(yaml: Document, id: string) {
-  yaml.commentBefore = [
+  dist.commentBefore = [
     `Generated from: ${id}.yml`,
     `Do not edit this file by hand. Edit the source file instead!`,
   ]
     .map((line) => ` ${line}`)
     .join("\n");
+
+  if (!source.status) {
+    dist.set("status", computedStatus);
+  }
+
+  if (!source.compat_features) {
+    dist.set("compat_features", taggedCompatFeatures);
+  }
+
+  return dist;
 }
 
 const compat = new Compat();
@@ -171,43 +154,6 @@ const tagsToFeatures: Map<string, Feature[]> = (() => {
   return map;
 })();
 
-function warnOnNeedlessOverrides(id, overridden, generated) {
-  if (overridden.compatFeatures?.length && generated.compatFeatures?.length) {
-    if (
-      isDeepStrictEqual(
-        [...overridden.compatFeatures].sort(),
-        [...generated.compatFeatures].sort(),
-      )
-    ) {
-      logger.warn(
-        `${id}: compat_features override matches tags in @mdn/browser-compat-data. Consider deleting this override.`,
-      );
-    }
-  }
-
-  if (
-    overridden.status &&
-    generated.statusByCompatFeaturesOverride &&
-    isDeepStrictEqual(
-      overridden.status,
-      generated.statusByCompatFeaturesOverride,
-    )
-  ) {
-    logger.warn(
-      `${id}: status override matches generated status from compat_features override. Consider deleting this override.`,
-    );
-  }
-  if (
-    overridden.status &&
-    generated.status &&
-    isDeepStrictEqual(overridden.status, generated.status)
-  ) {
-    logger.warn(
-      `${id}: status override matches generated status from tags. Consider deleting this override.`,
-    );
-  }
-}
-
 function main() {
   const filePaths = argv.paths.flatMap((fileOrDirectory) => {
     if (fs.statSync(fileOrDirectory).isDirectory()) {