Skip to content

Conversation

@ddbeck
Copy link
Collaborator

@ddbeck ddbeck commented Jul 9, 2024

The main thing this does is gives us a way to quickly and unambiguously report true, null, or false for unqualified support relationship between a release and a feature. There's some untidiness to clean up along the way. I'll self-review with commentary.

Comment on lines 16 to 27
// TODO:
// let lastInitial: Release | undefined;
// let lastInitialBoundary: "" | "≤" | undefined;

// const reverseChronological = b.releases.slice().reverse();
// let previousRelease: string | undefined;
// for (let index = 0; index < reverseChronological.length; index++) {
// const release = reverseChronological[index] as Release;
// const current = feature.flatSupportedIn(release);

// // Check if current has changed, etc.
// }
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 just to show where I'm going with this: walk backward through releases, then find the support-to-non-support boundary. If we stepped into a null, then that's ≤X.

Comment on lines 73 to 108
describe("flatSupportedIn()", function () {
it("returns true for unqualified support", function () {
const cr = browser("chrome");
assert.equal(
feature("api.Attr").flatSupportedIn(cr.version("100")),
true,
);
});

it("returns false for qualified support", function () {
const cr = browser("chrome");
assert.equal(
feature("css.properties.line-clamp").flatSupportedIn(
cr.version("100"),
),
false,
); // { version_added: "…", "prefix": "-webkit-" }
});

it("returns false for wholly unsupported", function () {
const fx = browser("firefox");
assert.equal(
feature("api.Accelerometer").flatSupportedIn(fx.current()),
false,
); // { version_added: false }
});

it("returns null for unknown support", function () {
const edge = browser("edge");
const f = feature("svg.elements.animate"); // { version_added: "≤79" }

assert.equal(f.flatSupportedIn(edge.version("12")), null);
assert.equal(f.flatSupportedIn(edge.version("79")), true);
assert.equal(f.flatSupportedIn(edge.version("80")), 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.

This is the flat version, which ignores prefixes and partials.

Comment on lines 110 to 152
describe("supportedIn()", function () {
it("returns support for features supported with and without qualification", function () {
const compat = new Compat();
const cr = browser("chrome");

// { version_added: "…" }
const bgColor = feature("css.properties.background-color").supportedIn(
cr.version("100"),
);
assert.equal(bgColor.length, 1);
assert.equal(bgColor[0]?.supported, true);

// { version_added: "…", prefix: "-webkit-" }
const lineClamp = feature("css.properties.line-clamp").supportedIn(
cr.version("100"),
);
assert.equal(lineClamp.length, 1);
assert.equal(lineClamp[0]?.supported, true);
assert.equal(lineClamp[0]?.qualifications?.prefix, "-webkit-");
});

it("returns mixed results for (un)prefixedfeatures", function () {
const fx = browser("firefox");
const actual = feature(
"css.types.image.gradient.repeating-linear-gradient",
).supportedIn(fx.version("100"));
assert.equal(actual.length, 3); // unprefixed, -moz-, and -webkit-
assert(actual.some((s) => s.supported && "qualifications" in s));
assert(actual.some((s) => s.supported && !("qualifications" in s)));
});

it("returns unknown support before version ranges", function () {
const edge = browser("edge");
const f = feature("svg.elements.animate");
const unknown = f.supportedIn(edge.version("12"));
assert.equal(unknown.length, 1);
assert.equal(unknown[0]?.supported, null);

const known = f.supportedIn(edge.version("79"));
assert.equal(known.length, 1);
assert.equal(known[0]?.supported, 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.

To find good test cases for the flat version, I ended up needing an un-flat version. We can dump this if we like, if we think we won't use it. OTOH, I did use it to write test cases. A conundrum.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Unless it gets exported to compute-baseline users, let's have it. Although if the flat version is the one we'll use most, maybe give that the nice name and rename the other one to a longer name?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done with c07b62d.

Comment on lines +78 to +92
rawSupportStatements(browser: Browser): SimpleSupportStatement[] {
const support = this.data?.__compat?.support;
if (support === undefined) {
throw Error("This feature contains no __compat object.");
throw new Error("This feature contains no __compat object.");
}

const statementOrStatements = support[browser.id];

if (statementOrStatements === undefined) {
throw Error(`${this} contains no support data for ${browser.name}`);
throw new Error(`${this} contains no support data for ${browser.name}`);
}

const rawStatements = Array.isArray(statementOrStatements)
return Array.isArray(statementOrStatements)
? statementOrStatements
: [statementOrStatements];
}
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 happens in a few places, so I consolidated it.

Comment on lines +94 to +98
supportStatements(browser: Browser): SupportStatement[] {
return this.rawSupportStatements(browser).map((raw) =>
statement(raw, browser, this),
);
}
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 too happens in a few places, now consolidated.

Comment on lines +103 to -100
this.assertRealSupportStatement(s, release.browser);

if (!(s instanceof RealSupportStatement)) {
throw Error(
`${this.id} contains non-real values for ${browser.name}. Cannot expand support.`,
);
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 also happens in a couple of places, so I consolidated that too—it's at the end. I have no idea how to order things. 😬

Copy link
Collaborator

Choose a reason for hiding this comment

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

Order is hard. End is fine.

@ddbeck ddbeck marked this pull request as ready for review July 9, 2024 18:15
@ddbeck ddbeck requested a review from foolip July 9, 2024 18:15
Comment on lines +103 to -100
this.assertRealSupportStatement(s, release.browser);

if (!(s instanceof RealSupportStatement)) {
throw Error(
`${this.id} contains non-real values for ${browser.name}. Cannot expand support.`,
);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Order is hard. End is fine.

Comment on lines 110 to 152
describe("supportedIn()", function () {
it("returns support for features supported with and without qualification", function () {
const compat = new Compat();
const cr = browser("chrome");

// { version_added: "…" }
const bgColor = feature("css.properties.background-color").supportedIn(
cr.version("100"),
);
assert.equal(bgColor.length, 1);
assert.equal(bgColor[0]?.supported, true);

// { version_added: "…", prefix: "-webkit-" }
const lineClamp = feature("css.properties.line-clamp").supportedIn(
cr.version("100"),
);
assert.equal(lineClamp.length, 1);
assert.equal(lineClamp[0]?.supported, true);
assert.equal(lineClamp[0]?.qualifications?.prefix, "-webkit-");
});

it("returns mixed results for (un)prefixedfeatures", function () {
const fx = browser("firefox");
const actual = feature(
"css.types.image.gradient.repeating-linear-gradient",
).supportedIn(fx.version("100"));
assert.equal(actual.length, 3); // unprefixed, -moz-, and -webkit-
assert(actual.some((s) => s.supported && "qualifications" in s));
assert(actual.some((s) => s.supported && !("qualifications" in s)));
});

it("returns unknown support before version ranges", function () {
const edge = browser("edge");
const f = feature("svg.elements.animate");
const unknown = f.supportedIn(edge.version("12"));
assert.equal(unknown.length, 1);
assert.equal(unknown[0]?.supported, null);

const known = f.supportedIn(edge.version("79"));
assert.equal(known.length, 1);
assert.equal(known[0]?.supported, true);
});
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

Unless it gets exported to compute-baseline users, let's have it. Although if the flat version is the one we'll use most, maybe give that the nice name and rename the other one to a longer name?

ddbeck and others added 2 commits July 11, 2024 18:32
@ddbeck ddbeck merged commit ca0769d into web-platform-dx:main Jul 11, 2024
@ddbeck ddbeck deleted the cb-exploder-part-2 branch July 11, 2024 16:34
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