Skip to content

Commit

Permalink
fix(compat-table): refine Destructuring support status (#3179)
Browse files Browse the repository at this point in the history
  • Loading branch information
@ArrayZoneYour
ArrayZoneYour authored Jun 25, 2023
1 parent a7236e4 commit 50f78c1
Show file tree
Hide file tree
Showing 3 changed files with 69 additions and 9 deletions.
8 changes: 8 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -335,6 +335,14 @@
Previously using TypeScript experimental decorators combined with the `--mangle-props` setting could result in a crash, as the experimental decorator transform was not expecting a mangled property as a class member. This release fixes the crash so you can now combine both of these features together safely.
* Fix a bug in esbuild's compatibility table script ([#3179](https://github.com/evanw/esbuild/pull/3179))
Setting esbuild's `target` to a specific JavaScript engine tells esbuild to use the JavaScript syntax feature compatibility data from https://kangax.github.io/compat-table/es6/ for that engine to determine which syntax features to allow. However, esbuild's script that builds this internal compatibility table had a bug that incorrectly ignores tests for engines that still have outstanding implementation bugs which were never fixed. This change fixes this bug with the script.
The only case where this changed the information in esbuild's internal compatibility table is that the `hermes` target is marked as no longer supporting destructuring. This is because there is a failing destructuring-related test for Hermes on https://kangax.github.io/compat-table/es6/. If you want to use destructuring with Hermes anyway, you can pass `--supported:destructuring=true` to esbuild to override the `hermes` target and force esbuild to accept this syntax.
This fix was contributed by [@ArrayZoneYour](https://github.com/ArrayZoneYour).
## 0.18.4
* Bundling no longer unnecessarily transforms class syntax ([#1360](https://github.com/evanw/esbuild/issues/1360), [#1328](https://github.com/evanw/esbuild/issues/1328), [#1524](https://github.com/evanw/esbuild/issues/1524), [#2416](https://github.com/evanw/esbuild/issues/2416))
Expand Down
1 change: 0 additions & 1 deletion internal/compat/js_table.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -397,7 +397,6 @@ var jsTable = map[JSFeature]map[Engine][]versionRange{
Edge: {{start: v{18, 0, 0}}},
ES: {{start: v{2015, 0, 0}}},
Firefox: {{start: v{53, 0, 0}}},
Hermes: {{start: v{0, 7, 0}}},
IOS: {{start: v{10, 0, 0}}},
Node: {{start: v{6, 5, 0}}},
Opera: {{start: v{38, 0, 0}}},
Expand Down
69 changes: 61 additions & 8 deletions scripts/compat-table.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,20 @@ const engines = [
'rhino',
]

const testsToSkip = new Set([
// Safari supposedly doesn't throw an error for duplicate identifiers in
// a function parameter list. The failing test case looks like this:
//
// var f = function f([id, id]) { return id }
//
// However, this code will cause a compile error with esbuild so it's not
// possible to encounter this issue when running esbuild-generated code in
// Safari. I'm ignoring this test since Safari's destructuring otherwise
// works fine so destructuring shouldn't be forbidden when building for
// Safari.
'destructuring, parameters: duplicate identifier',
])

function getValueOfTest(value) {
// Handle values like this:
//
Expand All @@ -126,17 +140,25 @@ function mergeVersions(target, res, omit = []) {
// take the minimum version to find the boundary.
const lowestVersionMap = {}

// If the current feature target is not supported for all versions, we will
// tag version[target] = { unsupported: true } to exclude it even when other
// sub-features are supported.
const unsupportedEngines = new Set()

for (const key in res) {
if (getValueOfTest(res[key])) {
const match = /^([a-z_]+)[0-9_]+$/.exec(key)
if (match) {
const engine = match[1]
const match = /^([a-z_]+)[0-9_]+$/.exec(key)
if (match) {
const engine = match[1]
if (getValueOfTest(res[key])) {
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
unsupportedEngines.delete(engine)
}
}
} else if (engines.indexOf(engine) >= 0 && !lowestVersionMap[engine]) {
unsupportedEngines.add(engine)
}
}
}
Expand All @@ -145,7 +167,14 @@ function mergeVersions(target, res, omit = []) {
// support a given feature if the version is greater than the maximum version
// for all subtests. This is the inverse of the minimum test below.
const highestVersionMap = versions[target] || (versions[target] = {})
for (const engine of unsupportedEngines) {
versions[target][engine] = [{ start: null, end: null, unsupported: true }]
}
for (const engine in lowestVersionMap) {
if (highestVersionMap[engine] && highestVersionMap[engine].unsupported) {
// Ignore unsupported engines
continue
}
const version = lowestVersionMap[engine]
if (!highestVersionMap[engine] || compareVersions({ version }, { version: highestVersionMap[engine][0].start }) > 0) {
highestVersionMap[engine] = [{ start: version, end: null }]
Expand Down Expand Up @@ -397,19 +426,27 @@ for (const test of [...es5.tests, ...es6.tests, ...stage4.tests, ...stage1to3.te
const res = {}

// Intersect all subtests (so a key is only true if it's true in all subtests)
for (const subtest of test.subtests)
for (const subtest of test.subtests) {
if (testsToSkip.has(`${test.name}: ${subtest.name}`))
continue
for (const key in subtest.res)
res[key] = true
for (const subtest of test.subtests)
}
for (const subtest of test.subtests) {
if (testsToSkip.has(`${test.name}: ${subtest.name}`))
continue
for (const key in res)
res[key] &&= getValueOfTest(subtest.res[key] ?? false)
}

mergeVersions(feature.target, res, omitNode)
} else {
mergeVersions(feature.target, test.res, omitNode)
}
} else if (test.subtests) {
for (const subtest of test.subtests) {
if (testsToSkip.has(`${test.name}: ${subtest.name}`))
continue
const feature = features[`${test.name}: ${subtest.name}`]
if (feature) {
feature.found = true
Expand Down Expand Up @@ -468,18 +505,26 @@ for (const test of [...es5.tests, ...es6.tests, ...stage4.tests, ...stage1to3.te
feature.found = true
if (test.subtests) {
const res = {}
for (const subtest of Object.values(test.subtests))
for (const subtest of Object.values(test.subtests)) {
if (testsToSkip.has(`${test.name}: ${subtest.name}`))
continue
for (const key in subtest.res)
res[key] = true
for (const subtest of Object.values(test.subtests))
}
for (const subtest of Object.values(test.subtests)) {
if (testsToSkip.has(`${test.name}: ${subtest.name}`))
continue
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)) {
if (testsToSkip.has(`${test.name}: ${subtest.name}`))
continue
const feature = features[`${test.name}: ${subtest.name}`]
if (feature) {
if (feature.target === 'ObjectAccessors') {
Expand All @@ -499,6 +544,14 @@ for (const feature in features) {
}
}

for (const target in versions) {
for (const engine in versions[target]) {
if (versions[target][engine][0] && versions[target][engine][0].unsupported) {
delete versions[target][engine]
}
}
}

function upper(text) {
if (text === 'es' || text === 'ios' || text === 'ie') return text.toUpperCase()
return text[0].toUpperCase() + text.slice(1)
Expand Down

0 comments on commit 50f78c1

Please sign in to comment.