Skip to content
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.

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

Add extended JSON file build output #1407

Merged
merged 8 commits into from
Jul 30, 2024
Merged

Conversation

ddbeck
Copy link
Collaborator

@ddbeck ddbeck commented Jul 16, 2024

This is a step toward #685 and #1316.

Unlike the regular npm package, this artifact includes computed statuses for each compat key, if a feature has a status and compat_features. This has also been known casually as the "fat" JSON artifact. This is intended for consumers that cannot or do not wish to use compute-baseline.

To make this artifact, use npm run build:extended.

@ddbeck ddbeck requested a review from foolip July 16, 2024 15:29
scripts/build.ts Outdated
@@ -39,3 +45,25 @@ function buildPackage() {
encoding: "utf-8",
});
}

function buildExtendedJSON() {
// TODO: Validate the resulting JSON against a schema.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you do this locally? I suspect that it doesn't match the schema. See below.

scripts/build.ts Outdated
const by_compat_key = {};

for (const key of featureData.compat_features) {
by_compat_key[key] = { status: getStatus(id, key) };
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this has to be unwrapped to match the schema:

Suggested change
by_compat_key[key] = { status: getStatus(id, key) };
by_compat_key[key] = getStatus(id, key);

scripts/build.ts Outdated Show resolved Hide resolved
"build": "tsx scripts/build.ts package",
"dist": "tsx scripts/dist.ts",
"feature-init": "tsx scripts/feature-init.ts",
"format": "npx prettier --write .",
"schema:write": "npm run schema -- --out ./schemas/data.schema.json",
"schema": "ts-json-schema-generator --tsconfig ./tsconfig.json --path ./types.ts",
"schema": "ts-json-schema-generator --tsconfig ./tsconfig.json --path ./types.ts --type=WebFeaturesData",
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This bit is pretty serious:

I figured out what that top-level $ref was for. Without it, it was just a list of definitions without asserting anything using those definitions. Without this option, any object will pass validation against our schema. 🙄

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh... let's put it back!

Comment on lines +40 to -24
if (!valid(data)) {
logger.error("Data failed schema validation. No package built.");
process.exit(1);
}

const json = stringify(data);
// TODO: Validate the resulting JSON against a schema.
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since I had to figure out how to validate the built JSON against the schema for the extended JSON, I did the same for the packaged one.

Comment on lines +91 to +94
if (!valid) {
// TODO: turn on strictNullChecks, fix all the errors, and replace this with:
// const errors = validate.errors;
const errors = (valid as any).errors as DefinedError[];
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a bit ugly because ajv can't produce (useful) types without strictNullChecks set to true in tsconfig.json. Fixable, but it requires touching a dozen files not germane to this PR.

Comment on lines +7 to +8
export function validate(data: any) {
const ajv = new Ajv({ allErrors: true, allowUnionTypes: true });
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I consolidated compiling the schema here, so that we can have one place with settings (and to check that we don't do something foolish like validate nothing at all).

@foolip foolip merged commit e77f25e into web-platform-dx:main Jul 30, 2024
3 checks passed
@ddbeck ddbeck deleted the fat-json branch August 5, 2024 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants