Custom compat tables #1791

p-bakker · 2022-02-12T16:47:32Z

As discussed in #1790 a prototype to allow creating custom compat tables

Usage node build environments=./custom.json excludecurrent

With custom.json being a custom environments file, for example having the following content:

{
  "rhino1_7_13": {
    "full": "Rhino 1.7.13",
    "short": "Rhino 1.7.13",
    "family": "Rhino",
    "platformtype": "engine",
    "release": "2020-09-02"
  },
  "rhino1_7_14": {
    "full": "Rhino 1.7.14",
    "short": "Rhino 1.7.14",
    "family": "Rhino",
    "platformtype": "engine",
    "release": "2022-01-06"
  }
}

This PR is a draft, just to get the discusion going. Needs cleanup + more:

Hide engines from the legenda that aren't used in the provided environments file
Hide unstable/obsolete checkboxes if the provided environments file doesn't contain any unstable/obsolete browsers
Remove sort by Engine Type, if only one engine type is is used with the environments file
Remove entire platformType header if the browsers in the environments file are all of the same platformType

One question: the code in build.js isn't using modern JavaScript features. Is that on purpose?

Closes #1790
Closes #1198

ljharb

seems reasonable so far

ljharb · 2022-02-12T17:03:55Z

build.js

+process.argv.slice(2).forEach(function(arg) {
+	var parts = String(arg).toLowerCase().split('=');
+
+	switch (parts[0]) {
+		case 'compilers':
+			useCompilers = true;
+			break;
+		case 'environments':
+			environments = require(parts[1]);
+			break;
+		case 'excludecurrent':
+			excludeCurrentBrowser = true;
+			break;
+	}
+});


let's use an args parser instead of rolling our own - https://www.npmjs.com/package/@pkgjs/parseargs will eventually be built into node, so that might be a good choice

Looking at the example for the parseargs package, what would be your preferred setup?

keep compilers as a positional?

excludecurrent as a flag? If so, suggestions as to which one? -e?

So something like -e --environments=./custom.json compilers?

That seems fine to me (altho maybe x for exclude)

Using @pkgjs/parseargs means upping the minimal NodeJS version for running build.js to NodeJS 15(.14.0), as the package makes use of ??= assignment operator.

That acceptible?

Yes, i think requiring node 16 is fine.

ljharb · 2022-02-12T17:05:40Z

build.js

+			useCompilers = true;
+			break;
+		case 'environments':
+			environments = require(parts[1]);


dynamic requires should be avoided; can we use fs.readFileSync and JSON.parse instead of require, since this is just json?

ljharb · 2022-02-12T17:05:53Z

build.js

+	}
+});
+
+if (!environments) environments = require('./environments');


this seems like it could be handled as a default in the args parser

I assume you mean a default value when destructuring the returnvalue from parseArgs(argv, options);, as I don't see any way to provide defaults to the parseArgs method.

If so, is it ok to use stuff like desctructuring in build.js, given that pretty much all the code in there now looks like ES5

Yes, that’s fine with me; the runtime must be ES3 but the build script can use modern node.

p-bakker · 2022-02-13T12:37:58Z

Having to make some adjustments to the es*/skeleton.html files, I'm wondering if I should be bothered to look at the es*/compiler-skeleton.html files as well.

Are they still being used? They don't seem to be hosted on https://kangax.github.io/compat-table/

p-bakker · 2022-02-14T11:45:15Z

@ljharb appreciate it if you could have another look at the direction of this PR: in 35bd089 I added the option to augment the testresults already available in this repo, as to allow supplying testdata, for example for nightlies of environments that are already part of this repo or to supply test results for environments that aren't in this repo (yet)

Also updated the readme with the new options and addresses previous review comments

If this all still looks good, last step will be to hide some more things form the UI, based on which environments are included

ljharb

Seems pretty straightforward.

It'd probably be good to have separate commits here; one for the "exclude current browser" option, etc.

ljharb · 2022-02-18T20:08:15Z

build-utils/interpolate-all-results.js

@@ -1,14 +1,41 @@
 var parseEnvsVersions = require("./parse-envs-versions");

-module.exports = function interpolateAllResults(tests, envs) {
+module.exports = function interpolateAllResults(tests, envs, customResults = {}) {


can you elaborate on what customResults is for?

This is to be able to supply build.js with test results for browser/browserVersions that aren't part of this repo.

As the test results aren't in json files, but in an exported object in the data-esXxxx.js files, i couldn't come up with another way to provide builds.js with results for for example a nightly.

Also see the changes I made to the readme, in which I (tried to) explain how this would be used.

BTW: did you see my question about whether the es*/compiler-skeleton.html are still being user? I think they are used when using the compilers argument, but it looks to me that the resulting pages aren't hosted anywhere, so might this all be obsolete stuff that could be removed?

What about instead providing an alternative list of browsers/browserVersions?

ok so "customResults" is more like "additionalResults"? it seems like maybe it'd be better for the caller to compose data-esX itself and make its own result object.

i have no idea about the compiler-skeleton stuff; before removing it, i'd want to know what it was for and why it's not being used anymore.

What about instead providing an alternative list of browsers/browserVersions?

Not sure I follow: that is what what the environments param allow you to do

ok so "customResults" is more like "additionalResults"?

correct

it seems like maybe it'd be better for the caller to compose data-esX itself and make its own result object.

I did look at such an approach, but as the test result data is inside the data-es*.js modules, I don't see a way to 'inject' additional testresults into those, except manually or by basically maintaining a modified implementation all the data-es*.js modules, which is something I don't like, because that is verly likely to get out of sync.

Another idea I had was just to refactor all the data-es*.js files to read their test results from another json file, instead of the test results being stored inside the module. But I was thinking that might break existing tooling or process to update the restresults

i have no idea about the compiler-skeleton stuff; before removing it, i'd want to know what it was for and why it's not being used anymore.

I haven't gotten to the bottom of it, but I think the es*/compiler-skeleton.html files are used to create .dedicated html files output files for just compilers. I think it may be that somewhere in the past, the currently hosted index.html files did not contain compiler results and they were hosted on separate pages. But it doesn't look like those separate compiler pages are still generated and hosted.

- introduce environments commandline param - make platformType headers colspan dynamic based on # on browser - hide platformType header if it doesn't contain any browsers

- introduce excludecurrent commandline param - disable running the tests in the browser - hide current browser columns

As to not have extremely wide columns on custom compat tbales that only show a few browsers

p-bakker · 2023-10-13T19:24:54Z

Closing this PR in favor of #1881 and yet to create follow-up PRS that offer a cleaner, more versatile approach

ljharb reviewed Feb 12, 2022

View reviewed changes

p-bakker requested a review from ljharb February 18, 2022 20:04

ljharb reviewed Feb 18, 2022

View reviewed changes

This comment was marked as spam.

Sign in to view

p-bakker added 8 commits October 6, 2023 08:32

Prepare for additional commandline arguments

83c1a22

Allow specifying custom environments.json on commandline

b780cdc

- introduce environments commandline param - make platformType headers colspan dynamic based on # on browser - hide platformType header if it doesn't contain any browsers

Make showing current browser results optional (during build)

24e5597

- introduce excludecurrent commandline param - disable running the tests in the browser - hide current browser columns

Use @pkgjs/parseargs for commandline arguments processing

bee3394

Add --customResults option to augment testresults

bac92a3

Use modern JS on new code

8b7d546

Don't force full width on table

c132907

As to not have extremely wide columns on custom compat tbales that only show a few browsers

Fix to hiding of current browser column & test running

a0634c8

p-bakker force-pushed the custom-compat-tables branch from f89167e to a0634c8 Compare October 6, 2023 08:47

This was referenced Oct 6, 2023

Externalize results from tests #1879

Open

Add results for GraalVM 21.3.3 and GraalVM 22.2 #1827

Merged

p-bakker closed this Oct 13, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Custom compat tables #1791

Custom compat tables #1791

p-bakker commented Feb 12, 2022 •

edited

ljharb left a comment

ljharb Feb 12, 2022

p-bakker Feb 12, 2022

ljharb Feb 12, 2022

p-bakker Feb 13, 2022

ljharb Feb 13, 2022

ljharb Feb 12, 2022

ljharb Feb 12, 2022

p-bakker Feb 12, 2022

ljharb Feb 12, 2022

p-bakker commented Feb 13, 2022

p-bakker commented Feb 14, 2022 •

edited

ljharb left a comment

ljharb Feb 18, 2022

p-bakker Feb 18, 2022

ljharb Feb 18, 2022

p-bakker Feb 18, 2022

This comment was marked as spam.

p-bakker commented Oct 13, 2023

Custom compat tables #1791

Custom compat tables #1791

Conversation

p-bakker commented Feb 12, 2022 • edited

ljharb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

p-bakker commented Feb 13, 2022

p-bakker commented Feb 14, 2022 • edited

ljharb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

This comment was marked as spam.

p-bakker commented Oct 13, 2023

p-bakker commented Feb 12, 2022 •

edited

p-bakker commented Feb 14, 2022 •

edited