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

Use node-compt-table #2940

Closed
Conaclos opened this issue Feb 20, 2023 · 5 comments
Closed

Use node-compt-table #2940

Conaclos opened this issue Feb 20, 2023 · 5 comments

Comments

@Conaclos
Copy link

Hi!

I was testing esbuild's target option and I noticed some gap in the compat table.
For instance node 14.5 supports optional chaining.

This is due to missing data on kangax's compat-table.
Node green is powered by a fork of the work of kangax: node-compat-table. It is dedicated to nodejs and have more accurate data.

This could be really nice to leverage this data for improving esbuild 's compat table.

@evanw
Copy link
Owner

evanw commented Feb 21, 2023

I looked into this. Here are my observations:

  • The first issue in that repo is titled Node.Green will soon stop working... with comments that indicate it has indeed stopped working, so doing the work to integrate this data source isn't necessarily that helpful.

  • According to https://node.green/, node 14.5 doesn't support optional chaining. It supports some forms of it but not all forms of it, so esbuild considers it unsupported and will still transpile it. Considering it to be supported in node 14.5 and not transpiling it would be incorrect because then some forms of it will be broken. So integrating this data source also won't help you accomplish your goal if your goal is avoiding the transpilation of optional chaining for node 14.5.

  • The data sources in node-compat-table are somewhat messy. For example, I have discovered that some of the strings contain embedded <code> tags while others don't, which makes matching them up more difficult. Some of the earlier node versions also use completely different tests (such as async functions›basic support in v0.12.17 vs. async functions›return in v0.12.18) which makes it hard to merge the data.

  • The data sources are missing anything about ES5 features. The important ones for esbuild are get and set accessor methods.

Here are the changes that would be applied if esbuild took in this data:

  • ClassPrivateBrandCheck would move earlier from v16.9 to v16.4

    I hand-checked this one with (new (class X{#x;foo(){return #x in this}})).foo() and v16.4 supports it while v16.3 does not, so this seems correct. This means esbuild would forbid class private brand checks in fewer cases than before.

  • Hashbang would move later from v12.0 to v12.5

    I hand-checked this one with !eval('#!/wash/your/hands') and v12.5 supports it while v12.4 doesn't, so this seems correct. This means esbuild would erase hashbangs from its output in more cases than before.

  • OptionalChain would move earlier from v16.9 to v16.1

    I hand-checked this one with the spread parameters after optional chaining test and v16.1 supports it while v16.0 doesn't, so this seems correct. This means esbuild would transpile optional chaining in fewer cases than before.

  • TemplateLiteral would move later from v4.0 to v10.0

    This means esbuild would transpile template literals in more cases than before. This appears to be necessary to work around a bug in node <v10 where (x=>x)`x` === (x=>x)`x` is incorrectly true.

So thank you for the pointer. This was a useful exercise, and I can use it to correct esbuild's data. But given that node-compat-table is now a dead data source, I think it's sufficient just to make the above four corrections instead of committing the changes for esbuild to pull from node-compat-table directly.

For my own future reference, here's some WIP code to pull from node-compat-table:
diff --git a/Makefile b/Makefile
index 640aa9e8..3a20f670 100644
--- a/Makefile
+++ b/Makefile
@@ -624,7 +624,11 @@ github/compat-table:
 	mkdir -p github/compat-table
 	git clone --depth 1 https://github.com/kangax/compat-table.git github/compat-table
 
-compat-table: | github/compat-table
+github/node-compat-table:
+	mkdir -p github/node-compat-table
+	git clone --depth 1 https://github.com/williamkapke/node-compat-table.git github/node-compat-table
+
+compat-table: | github/compat-table github/node-compat-table
 	node scripts/compat-table.js
 
 ################################################################################
diff --git a/scripts/compat-table.js b/scripts/compat-table.js
index 43d15c52..dcecaa9b 100644
--- a/scripts/compat-table.js
+++ b/scripts/compat-table.js
@@ -119,7 +119,7 @@ function getValueOfTest(value) {
   return value === true
 }
 
-function mergeVersions(target, res) {
+function mergeVersions(target, res, omit = []) {
   // The original data set will contain something like "chrome44: true" for a
   // given feature. And the interpolation script will expand this to something
   // like "chrome44: true, chrome45: true, chrome46: true, ..." so we want to
@@ -131,7 +131,7 @@ function mergeVersions(target, res) {
       const match = /^([a-z_]+)[0-9_]+$/.exec(key)
       if (match) {
         const engine = match[1]
-        if (engines.indexOf(engine) >= 0) {
+        if (engines.indexOf(engine) >= 0 && omit.indexOf(engine) < 0) {
           const version = parseEnvsVersions({ [key]: true })[engine][0].version
           if (!lowestVersionMap[engine] || compareVersions({ version }, { version: lowestVersionMap[engine] }) < 0) {
             lowestVersionMap[engine] = version
@@ -336,6 +336,8 @@ mergeVersions('RegexpMatchIndices', {
   safari15: true,
 })
 
+const omit = ['node']
+
 for (const test of [...es5.tests, ...es6.tests, ...stage4.tests, ...stage1to3.tests]) {
   const feature = features[test.name]
   if (feature) {
@@ -351,21 +353,94 @@ for (const test of [...es5.tests, ...es6.tests, ...stage4.tests, ...stage1to3.te
         for (const key in res)
           res[key] &&= getValueOfTest(subtest.res[key] ?? false)
 
-      mergeVersions(feature.target, res)
+      mergeVersions(feature.target, res, omit)
     } else {
-      mergeVersions(feature.target, test.res)
+      mergeVersions(feature.target, test.res, omit)
     }
   } else if (test.subtests) {
     for (const subtest of test.subtests) {
       const feature = features[`${test.name}: ${subtest.name}`]
       if (feature) {
         feature.found = true
-        mergeVersions(feature.target, subtest.res)
+        mergeVersions(feature.target, subtest.res, omit)
       }
     }
   }
 }
 
+// Node compatibility data is handled separately
+function handleNodeData() {
+  const nodeCompatTableDir = path.join(__dirname, '../github/node-compat-table/results/v8')
+  const reformattedTestResults = {}
+
+  // Format the data like the kangax table
+  for (const entry of fs.readdirSync(nodeCompatTableDir)) {
+    // Note: this omits data for the "0.x.y" releases because the data isn't clean
+    const match = /^([1-9]\d*\.\d+\.\d+)\.json$/.exec(entry)
+    if (match) {
+      const version = 'node' + match[1].replace(/\./g, '_')
+      const jsonPath = path.join(nodeCompatTableDir, entry)
+      const json = JSON.parse(fs.readFileSync(jsonPath, 'utf8'))
+      for (const key in json) {
+        if (key.startsWith('ES')) {
+          const object = json[key]
+          for (const key in object) {
+            const testResult = object[key]
+            const split = key.replace('<code>', '').replace('</code>', '').split('›')
+            if (split.length === 2) {
+              const test = reformattedTestResults[split[1]] || (reformattedTestResults[split[1]] = { name: split[1] })
+              const res = test.res || (test.res = {})
+              res[version] = testResult
+            } else if (split.length === 3) {
+              const test = reformattedTestResults[split[1]] || (reformattedTestResults[split[1]] = { name: split[1] })
+              const subtests = test.subtests || (test.subtests = {})
+              const subtest = subtests[split[2]] || (subtests[split[2]] = { name: split[2] })
+              const res = subtest.res || (subtest.res = {})
+              res[version] = testResult
+            }
+          }
+        }
+      }
+    }
+  }
+
+  for (const test of Object.values(reformattedTestResults)) {
+    const feature = features[test.name]
+    if (feature) {
+      feature.found = true
+      if (test.subtests) {
+        const res = {}
+
+        for (const subtest of Object.values(test.subtests))
+          for (const key in subtest.res)
+            res[key] = true
+        for (const subtest of Object.values(test.subtests))
+          for (const key in res)
+            res[key] &&= getValueOfTest(subtest.res[key] ?? false)
+
+        mergeVersions(feature.target, res)
+      } else {
+        mergeVersions(feature.target, test.res)
+      }
+    } else if (test.subtests) {
+      for (const subtest of Object.values(test.subtests)) {
+        const feature = features[`${test.name}: ${subtest.name}`]
+        if (feature) {
+          feature.found = true
+          mergeVersions(feature.target, subtest.res)
+        }
+      }
+    }
+  }
+}
+
+handleNodeData()
+
 for (const feature in features) {
   if (!features[feature].found) {
     throw new Error(`Did not find ${feature}`)

@jakebailey
Copy link

With williamkapke/node-compat-table#85, the data should start flowing again, if that's helpful.

@evanw
Copy link
Owner

evanw commented Mar 22, 2023

Thanks for the heads up! I'll subscribe to that PR.

@evanw
Copy link
Owner

evanw commented Mar 25, 2023

It has now landed, so I have integrated that data source. Thanks for your work on it. There were no changes when I integrated it, presumably due to my manual work earlier in this thread, but this integration should make keeping this up to date easier in the future.

@jakebailey
Copy link

Excellent! I learned about node.green's out-of-dateness from this thread, so I'm glad it was noticed and happy to help.

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

No branches or pull requests

3 participants