fix #1772: discontiguous ranges in support matrix

CHANGELOG.md

@@ -1,5 +1,15 @@
 # Changelog
 
+## Unreleased
+
+* Fix dynamic `import()` on node 12.20+ ([#1772](https://github.com/evanw/esbuild/issues/1772))
+
+    When you use flags such as `--target=node12.20`, esbuild uses that version number to see what features the target environment supports. This consults an internal table that stores which target environments are supported for each feature. For example, `import(x)` is changed into `Promise.resolve().then(() => require(x))` if dynamic `import` expressions are unsupported.
+
+    Previously esbuild's internal table only stored one version number, since features are rarely ever removed in newer versions of software. Either the target environment is before that version and the feature is unsupported, or the target environment is after that version and the feature is supported. This approach has work for all relevant features in all cases except for one: dynamic `import` support in node. This feature is supported in node 12.20.0 up to but not including node 13.0.0, and then is also supported in node 13.2.0 up. The feature table implementation has been changed to store an array of potentially discontiguous version ranges instead of one version number.
+
+    Up until now, esbuild used 13.2.0 as the lowest supported version number to avoid generating dynamic `import` expressions when targeting node versions that don't support it. But with this release, esbuild will now use the more accurate discontiguous version range in this case. This means dynamic `import` expressions can now be generated when targeting versions of node 12.20.0 up to but not including node 13.0.0.
+
 ## 0.13.13
 
 * Add more information about skipping `"main"` in `package.json` ([#1754](https://github.com/evanw/esbuild/issues/1754))

internal/compat/js_table.go

@@ -255,7 +255,7 @@ var jsTable = map[JSFeature]map[Engine][]versionRange{
 		ES:      {{start: v{2015, 0, 0}}},
 		Firefox: {{start: v{67, 0, 0}}},
 		IOS:     {{start: v{11, 0, 0}}},
-		Node:    {{start: v{13, 2, 0}}},
+		Node:    {{start: v{12, 20, 0}, end: v{13, 0, 0}}, {start: v{13, 2, 0}}},
 		Safari:  {{start: v{11, 1, 0}}},
 	},
 	ExponentOperator: {

scripts/compat-table.js

@@ -115,8 +115,8 @@ function mergeVersions(target, res) {
   const highestVersionMap = versions[target] || (versions[target] = {})
   for (const engine in lowestVersionMap) {
     const version = lowestVersionMap[engine]
-    if (!highestVersionMap[engine] || compareVersions({ version }, { version: highestVersionMap[engine] }) > 0) {
-      highestVersionMap[engine] = version
+    if (!highestVersionMap[engine] || compareVersions({ version }, { version: highestVersionMap[engine][0].start }) > 0) {
+      highestVersionMap[engine] = [{ start: version, end: null }]
     }
   }
 }
@@ -196,10 +196,21 @@ mergeVersions('DynamicImport', {
   edge79: true,
   firefox67: true,
   ios11: true,
-  node13_2: true, // From https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import
   safari11_1: true,
 })
 
+// This is a special case. Node added support for it to both v12.20+ and v13.2+
+// so the range is inconveniently discontiguous. Sources:
+//
+// - https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/import
+// - https://github.com/nodejs/node/pull/35950
+// - https://github.com/nodejs/node/pull/31974
+//
+versions.DynamicImport.node = [
+  { start: [12, 20], end: [13] },
+  { start: [13, 2] },
+]
+
 mergeVersions('ArbitraryModuleNamespaceNames', {
   // From https://github.com/tc39/ecma262/pull/2154#issuecomment-825201030
   chrome90: true,
@@ -274,7 +285,13 @@ function writeInnerMap(obj) {
   const keys = Object.keys(obj).sort()
   const maxLength = keys.reduce((a, b) => Math.max(a, b.length + 1), 0)
   if (keys.length === 0) return '{}'
-  return `{\n${keys.map(x => `\t\t${(upper(x) + ':').padEnd(maxLength)} {{start: v{${obj[x].concat(0, 0).slice(0, 3).join(', ')}}}},`).join('\n')}\n\t}`
+  return `{\n${keys.map(x => {
+    const items = obj[x].map(y => {
+      return `{start: v{${y.start.concat(0, 0).slice(0, 3).join(', ')
+        }}${y.end ? `, end: v{${y.end.concat(0, 0).slice(0, 3).join(', ')}}` : ''}}`
+    })
+    return `\t\t${(upper(x) + ':').padEnd(maxLength)} {${items.join(', ')}},`
+  }).join('\n')}\n\t}`
 }
 
 fs.writeFileSync(__dirname + '/../internal/compat/js_table.go',

scripts/js-api-tests.js

@@ -3897,31 +3897,59 @@ let transformTests = {
   },
 
   async dynamicImportString({ esbuild }) {
-    const { code: code1 } = await esbuild.transform(`import('foo')`, { target: 'chrome63' })
-    assert.strictEqual(code1, `import("foo");\n`)
+    const { code } = await esbuild.transform(`import('foo')`, { target: 'chrome63' })
+    assert.strictEqual(code, `import("foo");\n`)
   },
 
   async dynamicImportStringES6({ esbuild }) {
     const fromPromiseResolve = text => text.slice(text.indexOf('Promise.resolve'))
-    const { code: code2 } = await esbuild.transform(`import('foo')`, { target: 'chrome62' })
-    assert.strictEqual(fromPromiseResolve(code2), `Promise.resolve().then(() => __toModule(require("foo")));\n`)
+    const { code } = await esbuild.transform(`import('foo')`, { target: 'chrome62' })
+    assert.strictEqual(fromPromiseResolve(code), `Promise.resolve().then(() => __toModule(require("foo")));\n`)
   },
 
   async dynamicImportStringES5({ esbuild }) {
     const fromPromiseResolve = text => text.slice(text.indexOf('Promise.resolve'))
-    const { code: code3 } = await esbuild.transform(`import('foo')`, { target: 'chrome48' })
-    assert.strictEqual(fromPromiseResolve(code3), `Promise.resolve().then(function() {\n  return __toModule(require("foo"));\n});\n`)
+    const { code } = await esbuild.transform(`import('foo')`, { target: 'chrome48' })
+    assert.strictEqual(fromPromiseResolve(code), `Promise.resolve().then(function() {\n  return __toModule(require("foo"));\n});\n`)
   },
 
   async dynamicImportStringES5Minify({ esbuild }) {
     const fromPromiseResolve = text => text.slice(text.indexOf('Promise.resolve'))
-    const { code: code4 } = await esbuild.transform(`import('foo')`, { target: 'chrome48', minifyWhitespace: true })
-    assert.strictEqual(fromPromiseResolve(code4), `Promise.resolve().then(function(){return __toModule(require("foo"))});\n`)
+    const { code } = await esbuild.transform(`import('foo')`, { target: 'chrome48', minifyWhitespace: true })
+    assert.strictEqual(fromPromiseResolve(code), `Promise.resolve().then(function(){return __toModule(require("foo"))});\n`)
+  },
+
+  async dynamicImportStringNode12_19({ esbuild }) {
+    const fromPromiseResolve = text => text.slice(text.indexOf('Promise.resolve'))
+    const { code } = await esbuild.transform(`import('foo')`, { target: 'node12.19' })
+    assert.strictEqual(fromPromiseResolve(code), `Promise.resolve().then(() => __toModule(require("foo")));\n`)
+  },
+
+  async dynamicImportStringNode12_20({ esbuild }) {
+    const { code } = await esbuild.transform(`import('foo')`, { target: 'node12.20' })
+    assert.strictEqual(code, `import("foo");\n`)
+  },
+
+  async dynamicImportStringNode13({ esbuild }) {
+    const fromPromiseResolve = text => text.slice(text.indexOf('Promise.resolve'))
+    const { code } = await esbuild.transform(`import('foo')`, { target: 'node13' })
+    assert.strictEqual(fromPromiseResolve(code), `Promise.resolve().then(() => __toModule(require("foo")));\n`)
+  },
+
+  async dynamicImportStringNode13_1({ esbuild }) {
+    const fromPromiseResolve = text => text.slice(text.indexOf('Promise.resolve'))
+    const { code } = await esbuild.transform(`import('foo')`, { target: 'node13.1' })
+    assert.strictEqual(fromPromiseResolve(code), `Promise.resolve().then(() => __toModule(require("foo")));\n`)
+  },
+
+  async dynamicImportStringNode13_2({ esbuild }) {
+    const { code } = await esbuild.transform(`import('foo')`, { target: 'node13.2' })
+    assert.strictEqual(code, `import("foo");\n`)
   },
 
   async dynamicImportExpression({ esbuild }) {
-    const { code: code1 } = await esbuild.transform(`import(foo)`, { target: 'chrome63' })
-    assert.strictEqual(code1, `import(foo);\n`)
+    const { code } = await esbuild.transform(`import(foo)`, { target: 'chrome63' })
+    assert.strictEqual(code, `import(foo);\n`)
   },
 
   async dynamicImportExpressionES6({ esbuild }) {