`compute-baseline`: add method to expand a support statement into per-release support information #1312

ddbeck · 2024-07-03T16:40:55Z

This is another incremental step toward handling version ranges more completely, by taking a support statement and mapping it to every release that a browser has.

More specifically, this PR:

Adds toReleaseSupportMap() method to SupportStatements. It's very much like supportedBy(), except it has something to say about every version, not just the supporting versions. I've made some of supportedBy() reusable. Eventually, but not today, we'll rip out supportedBy() entirely.
Adds an inRange(start, end?) method to Releasess. This is a not-infrequent thing to check and remembering what >= 0 means is hard.
Remove some never-used code relating to ranges.
Handle bugged version_removed values in BCD. Basically, some values should never happen according to the schema, but are happening anyway. This won't be completely fixed until it gets fixed upstream.

The next step after this will be to reconcile multiple support statements at the feature level (i.e., filtering out "unsupported" entries when there's a competing "supported" entry) and do it across browsers (e.g., to create something like Map<Browser, Map<Release, Supported | Unsupported | UnknownSupport>>).

…-release support information

packages/compute-baseline/src/browser-compat-data/release.test.ts

foolip · 2024-07-04T12:13:57Z

packages/compute-baseline/src/browser-compat-data/supportStatements.ts

+   */
+  toReleaseSupportMap(): ReleaseSupportMap {
+    if (this.browser === undefined) {
+      throw Error("This support statement's browser is unknown.");


Suggested change

throw Error("This support statement's browser is unknown.");

throw new Error("This support statement's browser is unknown.");

foolip · 2024-07-04T12:14:14Z

packages/compute-baseline/src/browser-compat-data/supportStatements.ts

-  // returning `[this.browser.current()]` at least.
  supportedBy(): { release: Release; qualifications?: Qualifications }[] {
    if (this.browser === undefined) {
      throw Error("This support statement's browser is unknown.");


Suggested change

throw Error("This support statement's browser is unknown.");

throw new Error("This support statement's browser is unknown.");

This is a problem in a bunch of places. I'll open a follow-up PR to fix this across the board.

foolip · 2024-07-04T12:15:38Z

packages/compute-baseline/src/browser-compat-data/supportStatements.ts

+      throw Error("This support statement's browser is unknown.");
+    }
+
+    if (this.version_added === false || this.version_removed === true) {


If version_removed is true, support is unknown for everything except the version_added version.

foolip · 2024-07-04T12:17:28Z

packages/compute-baseline/src/browser-compat-data/supportStatements.ts

+    let startRanged = false;
+    if (this.version_added === true) {
+      startRanged = true;
+      start = this.browser.current();


This is correct, but if this code is actually used, our dist files will keep changing. Should we instead refuse to work with true values and wait for them to be fixed in BCD?

foolip · 2024-07-04T12:23:20Z

packages/compute-baseline/src/browser-compat-data/supportStatements.ts

+   * Expand this support statement into a `Map` from `Release` objects to objects
+   * describing whether the release is supporting, unsupporting, or unknown.
+   */
+  toReleaseSupportMap(): ReleaseSupportMap {


It looks like this expands a single support statement (not an array) into a map. That means the resulting map will always be simple, typically with an initial run of false values followed by a run or true values. Sometimes unknown support before a point, but never flip-flopping. Is a map really necessary to represent this, or should we just parse version_added and version_removed into a form that makes it easy to get support for a specific release?

foolip · 2024-07-04T12:24:12Z

packages/compute-baseline/src/browser-compat-data/supportStatements.ts

+
+    let end: Release | undefined;
+    if (this.version_removed) {
+      end = this.browser.version(this.version_removed);


What about ranges here?

foolip · 2024-07-04T12:25:29Z

packages/compute-baseline/src/browser-compat-data/supportStatements.ts

+      if (isQualified && r.inRange(start, end)) {
+        // Supported with qualifications
+        result.set(r, { supported: true, qualifications });
+      } else if (r.inRange(start, end)) {
+        // Supported without qualification
+        result.set(r, { supported: true });
+      } else if (startRanged && !r.inRange(start)) {
+        // Support unknown (before a ≤ version)
+        result.set(r, { supported: null });
+      } else {
+        // Unsupported (outside a hard range)
+        result.set(r, { supported: false });
+      }


How about putting this into a method to get support for a specific release? That's cheaper when the caller doesn't want to know about all releases.

ddbeck · 2024-07-08T11:57:01Z

packages/compute-baseline/src/browser-compat-data/supportStatements.ts

+   * Find out whether this support statement says a given browser release is
+   * supported (with or without qualifications), unsupported, or unknown.
+   */
+  supportedIn(release: Release): Supported | Unsupported | UnknownSupport {


@foolip I've refactored this as a method that just checks whether one release is supported. If one still wants to know the situation for every release, then that's a one-liner away:

new Map(browser.releases.map(r => [r, statement.supportedIn(r)]))

Also, doing this for any arbitrary support statement, while possible, was getting very tricky and hard to follow. Since we don't need this I decided it wasn't worth the effort, so it's only available on RealSupportStatements.

(I've kept some test cases and work from the all-values version, but it's not urgent, so I'll re-introduce that when there's an actual need or a spare couple of hours for it.)

foolip · 2024-07-08T14:19:35Z

packages/compute-baseline/src/browser-compat-data/release.test.ts

+      assert.throws(() => cr.version("50").inRange(fx.version("50")), Error);
+    });
+
+    it("handles closed ranges", function () {


Can you add a test for when the version equals the end version, to show the range is exclusive of the end point?

Done with 7c2bbad.

foolip · 2024-07-08T14:26:29Z

packages/compute-baseline/src/browser-compat-data/supportStatements.test.ts

+        assert.equal(ranged.supportedIn(cr.version("99")).supported, null);
+        assert.equal(ranged.supportedIn(cr.version("100")).supported, true);
+        assert.equal(ranged.supportedIn(cr.version("101")).supported, true);
+        assert.equal(ranged.supportedIn(cr.version("124")).supported, true);


Why not the 125 assert here, so it's the same asserts for unranged and ranged? (You might remove 101 if you want to reduce the number of rows.)

Done with 4e0110b.

foolip · 2024-07-08T14:28:14Z

packages/compute-baseline/src/browser-compat-data/supportStatements.test.ts

+        );
+
+        assert.equal(rangedEnd.supportedIn(cr.version("100")).supported, true);
+        assert.equal(rangedEnd.supportedIn(cr.version("124")).supported, null);


Can you also test 101 here to make it clear that there's just a single release we're sure about?

Done with 4e0110b.

packages/compute-baseline/src/browser-compat-data/supportStatements.test.ts

foolip · 2024-07-08T14:54:21Z

packages/compute-baseline/src/browser-compat-data/supportStatements.ts

+  return qualifications;
+}
+
+function isRangedVersion(s: any): s is string {


I've only seen this syntax once before, but does s is string really do the right thing if this method returns false? Won't TypeScript infer that the argument wasn't a string even though it may have been?

Yes, this really does the right thing. Type predicates are (by themselves) one-way: if the function returns true, then the type is satisfied. The inverse (false → s is not string) is not inferred. Demo.

AFAICT, even in general TypeScript doesn't actually track the list of types something isn't, just the set of remaining possible types.

Co-authored-by: Philip Jägenstedt <philip@foolip.org>

ddbeck added the package:compute-baseline label Jul 3, 2024

ddbeck requested a review from foolip July 3, 2024 16:40

ddbeck added 4 commits July 3, 2024 18:41

Make turning a statement into a list of qualifications reusable

3b28a74

Add convenience method for checking if a release is in a range

9f271ec

compute-baseline: add method to expand a support statement into per…

222fd82

…-release support information

Remove dead code relating to version ranges

5129251

ddbeck force-pushed the cb-exploder branch from 2b673cb to dfddd65 Compare July 3, 2024 16:41

Handle version_removed values not matching schema

fd53190

ddbeck force-pushed the cb-exploder branch from dfddd65 to fd53190 Compare July 3, 2024 16:43

Treat version_added: true like `version_added: "≤current"

c1124f9

foolip reviewed Jul 4, 2024

View reviewed changes

Luis-2305 approved these changes Jul 5, 2024

View reviewed changes

ddbeck added 2 commits July 5, 2024 10:50

Test comparing releases across browers (throws)

72dc14b

Refactor toSupportReleaseMap() to supportedIn(…)

1b41409

ddbeck force-pushed the cb-exploder branch from 3e98b1d to 1b41409 Compare July 8, 2024 11:52

ddbeck commented Jul 8, 2024

View reviewed changes

ddbeck requested a review from foolip July 8, 2024 11:59

foolip reviewed Jul 8, 2024

View reviewed changes

ddbeck mentioned this pull request Jul 9, 2024

Fix throwing non-new Errors and literals #1338

Merged

ddbeck and others added 3 commits July 9, 2024 14:25

Simplify test case for unsupported on version_added: false

3cfd35d

Co-authored-by: Philip Jägenstedt <philip@foolip.org>

Add test (and comments) for inRange end exclusivity

7c2bbad

Improve consistency/symmetry of tests

4e0110b

foolip approved these changes Jul 9, 2024

View reviewed changes

foolip merged commit f6171e4 into web-platform-dx:main Jul 9, 2024

ddbeck deleted the cb-exploder branch July 9, 2024 17:46

foolip mentioned this pull request Mar 12, 2025

Increment compute-baseline version to 0.3.0 #2764

Merged

	throw Error("This support statement's browser is unknown.");
	throw new Error("This support statement's browser is unknown.");

compute-baseline: add method to expand a support statement into per-release support information #1312

compute-baseline: add method to expand a support statement into per-release support information #1312

Uh oh!

Conversation

ddbeck commented Jul 3, 2024

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

`compute-baseline`: add method to expand a support statement into per-release support information #1312

`compute-baseline`: add method to expand a support statement into per-release support information #1312