w3c / spec-prod

GitHub Action to build ReSpec/Bikeshed specs, validate output and publish to GitHub pages or W3C
https://w3c.github.io/spec-prod/
MIT License
33 stars 19 forks source link

fix(validate/webidl): upgrade reffy #155

Closed dontcallmedom closed 6 months ago

sidvishnoi commented 1 year ago

Well that's a jump. The API is same, right?

dontcallmedom commented 1 year ago

hmm... I recall we did some breaking changes in the CLI, so this probably needs more careful checking; @tidoust would probably know best, but is away this week

sidvishnoi commented 1 year ago

I'll see if we can use package.json (and dependabot) for this. Via optional dependencies perhaps. Adding a CI for spec-prod itself is also on my TODO list.

tidoust commented 1 year ago

Breaking changes are documented in major release commits. Also see the -v5.0.0 commit which did not follow the same commit message convention.

One breaking change affects spec-prod: v6.0.0 separated the raw IDL and the parsed IDL in the result structure, which means that the following line needs to be updated to const idl = results[0]?.idl:

https://github.com/w3c/spec-prod/blob/29ee061d21418bd0ef6c3c94e824bb9f7bd15b74/src/validate-webidl.ts#L27

Other breaking changes should not affect Web IDL extraction.

sidvishnoi commented 6 months ago

Would someone be willing to update reffy further and handle the breaking changes?

sidvishnoi commented 6 months ago

I'll add a test infra for this project sometime! Until then, 🤞🏽

tidoust commented 6 months ago

@sidvishnoi, I rebased the PR and force-pushed an update that bumps Reffy to its latest major version, and makes the change I raised earlier.

As far as I can tell, that should be all that's needed (IDL extraction has not changed significantly in the past few years). Now, I have not checked the resulting code. If there's a simple way to test this before merging, that could prove useful...

sidvishnoi commented 6 months ago

Simplest way to test would be to create a fork of a repo, and use w3c/spec-prod@dontcallmedom-patch-1 in workflow.

sidvishnoi commented 6 months ago

I tested locally by curl -o test.html a spec, and running tsx src/validate-webidl.ts

diff --git a/src/validate-webidl.ts b/src/validate-webidl.ts
index 40d3c67..1946c41 100644
--- a/src/validate-webidl.ts
+++ b/src/validate-webidl.ts
@@ -4,17 +4,15 @@ import { BuildResult } from "./build.js";
 type Input = Pick<BuildResult, "dest" | "file">;

 if (module === require.main) {
-   if (yesOrNo(env("INPUTS_VALIDATE_WEBIDL")) === false) {
-       exit("Skipped", 0);
-   }
-
-   const input: Input = JSON.parse(env("OUTPUTS_BUILD"));
-   main(input).catch(err => exit(err.message || "Failed", err.code));
+   main({
+       dest: process.cwd(),
+       file: "test.html",
+   }).catch(err => exit(err.message || "Failed", err.code));
 }

 export default async function main({ dest, file }: Input) {
    console.log(`Validating Web IDL defined in ${file}...`);
-   await install("reffy@4");
+   await install("reffy@15");
    const { crawlSpecs } = require("reffy");

    const fileurl = new URL(file, `file://${dest}/`).href;
@@ -24,7 +22,7 @@ export default async function main({ dest, file }: Input) {
    );
    await rm(".cache", { recursive: true, force: true });

-   const idl = results[0]?.idl?.idl;
+   const idl = results[0]?.idl;
    if (!idl) {
        exit("No Web IDL found in spec, skipped validation", 0);
    }