Skip to content

Commit

Permalink
Implement @babel/plugin-bugfix-safari-id-destructuring-collision-in-f…
Browse files Browse the repository at this point in the history
…unction-expression (#13842)

* fix: register function expression id after params

* implement bugfix plugin

* add more testcases

* fix: do not skip pattern binding referencing id

* update compat-table

* add bugfix plugin to preset-env

* update Babel 8 test fixtures

* Update packages/babel-plugin-bugfix-safari-id-destructuring-collision-in-function-expression/README.md

* chore: bundle bugfix plugin

* address review comments

* add runtime version check

* update compat table

* fix syntax error

* update test fixtures

* revert bugfixes targets update

* update Babel 8 test fixtures
  • Loading branch information
JLHwung committed Oct 20, 2021
1 parent 780aa48 commit 29f697c
Show file tree
Hide file tree
Showing 195 changed files with 732 additions and 311 deletions.
1 change: 1 addition & 0 deletions Gulpfile.mjs
Expand Up @@ -465,6 +465,7 @@ const libBundles = [
"packages/babel-preset-typescript",
"packages/babel-helper-member-expression-to-functions",
"packages/babel-plugin-bugfix-v8-spread-parameters-in-optional-chaining",
"packages/babel-plugin-bugfix-safari-id-destructuring-collision-in-function-expression",
].map(src => ({
src,
format: "cjs",
Expand Down
20 changes: 11 additions & 9 deletions Makefile
Expand Up @@ -186,15 +186,17 @@ prepublish:
IS_PUBLISH=true $(MAKE) test

new-version-checklist:
# @echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"
# @echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"
# @echo "!!!!!! !!!!!!"
# @echo "!!!!!! Write any message that should !!!!!!"
# @echo "!!!!!! block the release here !!!!!!"
# @echo "!!!!!! !!!!!!"
# @echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"
# @echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"
# @exit 1
@echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"
@echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"
@echo "!!!!!! !!!!!!"
@echo "!!!!!! packages/babel-plugin-bugfix-safari-id-destructuring-collision-in-function-expression/src/index.ts"
@echo "!!!!!! replace \"api.assertVersion()\" to the latest published version"
@echo "!!!!!! packages/babel-preset-env/src/available-plugins.ts:"
@echo "!!!!!! replace minVersion[\"bugfix/transform-v8-spread-parameters-in-optional-chaining\"] to the latest published version"
@echo "!!!!!! !!!!!!"
@echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"
@echo "!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!!"
@exit 1

new-version:
$(MAKE) new-version-checklist
Expand Down
3 changes: 2 additions & 1 deletion packages/babel-compat-data/data/overlapping-plugins.json
Expand Up @@ -3,7 +3,8 @@
"bugfix/transform-async-arrows-in-class"
],
"transform-parameters": [
"bugfix/transform-edge-default-parameters"
"bugfix/transform-edge-default-parameters",
"bugfix/transform-safari-id-destructuring-collision-in-function-expression"
],
"transform-function-name": [
"bugfix/transform-edge-function-name"
Expand Down
14 changes: 13 additions & 1 deletion packages/babel-compat-data/data/plugin-bugfixes.json
Expand Up @@ -101,6 +101,16 @@
"rhino": "1.7.13",
"electron": "0.37"
},
"bugfix/transform-safari-id-destructuring-collision-in-function-expression": {
"chrome": "49",
"opera": "36",
"edge": "14",
"firefox": "2",
"node": "6",
"samsung": "5",
"rhino": "1.7.13",
"electron": "0.37"
},
"transform-template-literals": {
"chrome": "41",
"opera": "28",
Expand Down Expand Up @@ -135,8 +145,10 @@
"electron": "8.0"
},
"bugfix/transform-v8-spread-parameters-in-optional-chaining": {
"chrome": "91",
"firefox": "74",
"safari": "13.1",
"ios": "13.4"
"ios": "13.4",
"electron": "13.0"
}
}
18 changes: 12 additions & 6 deletions packages/babel-compat-data/data/plugins.json
@@ -1,11 +1,13 @@
{
"proposal-class-static-block": {
"chrome": "91",
"electron": "13.0"
"chrome": "94",
"firefox": "93"
},
"proposal-private-property-in-object": {
"chrome": "91",
"firefox": "90",
"safari": "15",
"ios": "15",
"electron": "13.0"
},
"proposal-class-properties": {
Expand All @@ -15,6 +17,7 @@
"firefox": "90",
"safari": "14.1",
"node": "12",
"ios": "15",
"samsung": "11",
"electron": "6.0"
},
Expand All @@ -25,6 +28,8 @@
"firefox": "90",
"safari": "15",
"node": "14.6",
"ios": "15",
"samsung": "14",
"electron": "10.0"
},
"proposal-numeric-separator": {
Expand All @@ -46,6 +51,7 @@
"safari": "14",
"node": "15",
"ios": "14",
"samsung": "14",
"electron": "10.0"
},
"proposal-nullish-coalescing-operator": {
Expand All @@ -60,9 +66,11 @@
"electron": "8.0"
},
"proposal-optional-chaining": {
"chrome": "91",
"firefox": "74",
"safari": "13.1",
"ios": "13.4"
"ios": "13.4",
"electron": "13.0"
},
"proposal-json-strings": {
"chrome": "66",
Expand Down Expand Up @@ -91,9 +99,7 @@
"opera": "36",
"edge": "18",
"firefox": "53",
"safari": "10",
"node": "6",
"ios": "10",
"samsung": "5",
"electron": "0.37"
},
Expand Down Expand Up @@ -379,7 +385,7 @@
"chrome": "46",
"opera": "33",
"edge": "14",
"firefox": "45",
"firefox": "41",
"safari": "10",
"node": "5",
"ios": "10",
Expand Down
4 changes: 4 additions & 0 deletions packages/babel-compat-data/scripts/data/plugin-bugfixes.js
Expand Up @@ -31,6 +31,10 @@ module.exports = {
],
replaces: "transform-block-scoping",
},
"bugfix/transform-safari-id-destructuring-collision-in-function-expression": {
features: ["destructuring, parameters / duplicate identifier"],
replaces: "transform-parameters",
},
"bugfix/transform-tagged-template-caching": {
features: ["template literals / TemplateStrings permanent caching"],
replaces: "transform-template-literals",
Expand Down
1 change: 1 addition & 0 deletions packages/babel-compat-data/scripts/data/plugin-features.js
Expand Up @@ -18,6 +18,7 @@ const es2015Parameter = {
"rest parameters",
"destructuring, parameters / aliased defaults, arrow function",
"destructuring, parameters / shorthand defaults, arrow function",
"destructuring, parameters / duplicate identifier",
],
},
};
Expand Down
@@ -1,7 +1,7 @@
#!/bin/bash
set -e

COMPAT_TABLE_COMMIT=63abfe227f4b9c6ef019efbbf059025b537b8511
COMPAT_TABLE_COMMIT=34b14f8eb016f8f2f3312adf35c53d6364742582
GIT_HEAD=build/compat-table/.git/HEAD

if [ -d "build/compat-table" ]; then
Expand Down
@@ -0,0 +1,3 @@
src
test
*.log
@@ -0,0 +1,19 @@
# @babel/plugin-bugfix-safari-id-destructuring-collision-in-function-expression

> Rename destructuring parameter to workaround a [Safari bug](https://bugs.webkit.org/show_bug.cgi?id=220517).
See our website [@babel/plugin-bugfix-safari-id-destructuring-collision-in-function-expression](https://babeljs.io/docs/en/babel-plugin-bugfix-safari-id-destructuring-collision-in-function-expression) for more information.

## Install

Using npm:

```sh
npm install --save-dev @babel/plugin-bugfix-safari-id-destructuring-collision-in-function-expression
```

or using yarn:

```sh
yarn add @babel/plugin-bugfix-safari-id-destructuring-collision-in-function-expression --dev
```
@@ -0,0 +1,40 @@
{
"name": "@babel/plugin-bugfix-safari-id-destructuring-collision-in-function-expression",
"version": "0.0.0",
"description": "Rename destructuring parameter to workaround https://bugs.webkit.org/show_bug.cgi?id=220517",
"repository": {
"type": "git",
"url": "https://github.com/babel/babel.git",
"directory": "packages/babel-plugin-bugfix-safari-id-destructuring-collision-in-function-expression"
},
"homepage": "https://babel.dev/docs/en/next/babel-plugin-bugfix-safari-id-destructuring-collision-in-function-expression",
"license": "MIT",
"publishConfig": {
"access": "public"
},
"main": "./lib/index.js",
"exports": {
".": [
"./lib/index.js"
]
},
"keywords": [
"babel-plugin",
"bugfix"
],
"dependencies": {
"@babel/helper-plugin-utils": "workspace:^7.14.5"
},
"peerDependencies": {
"@babel/core": "^7.0.0"
},
"devDependencies": {
"@babel/core": "workspace:*",
"@babel/helper-plugin-test-runner": "workspace:*",
"@babel/traverse": "workspace:*"
},
"engines": {
"node": ">=6.9.0"
},
"author": "The Babel Team (https://babel.dev/team)"
}
@@ -0,0 +1,25 @@
import { declare } from "@babel/helper-plugin-utils";
import type { PluginPass } from "@babel/core";
import type { Visitor } from "@babel/traverse";
import { shouldTransform } from "./util";

export default declare(api => {
api.assertVersion("^7.15.0");

return {
name: "plugin-bugfix-safari-id-destructuring-collision-in-function-expression",

visitor: {
FunctionExpression(path) {
const name = shouldTransform(path);
if (name) {
// Now we have (function a([a]) {})
const { scope } = path;
// invariant: path.node.id is always an Identifier here
const newParamName = scope.generateUid(name);
scope.rename(name, newParamName);
}
},
} as Visitor<PluginPass>,
};
});
@@ -0,0 +1,41 @@
import type { FunctionExpression } from "@babel/types";
import type { NodePath } from "@babel/traverse";

/**
* Check whether a function expression can be affected by
* https://bugs.webkit.org/show_bug.cgi?id=220517
* @param path The function expression NodePath
* @returns the name of function id if it should be transformed, otherwise returns false
*/
export function shouldTransform(
path: NodePath<FunctionExpression>,
): string | false {
const { node } = path;
const functionId = node.id;
if (!functionId) return false;

const name = functionId.name;
// On collision, `getOwnBinding` returns the param binding
// with the id binding be registered as constant violation
const paramNameBinding = path.scope.getOwnBinding(name);
const constantViolations = paramNameBinding.constantViolations;
if (constantViolations.length === 0) {
// the function scope has no such collided bindings
return false;
}
const firstViolation = constantViolations[0];

if (firstViolation.node !== node) {
// the violation does not happen in id
// e.g. (function a() { var a; })
return false;
}

if (paramNameBinding.identifier === paramNameBinding.path.node) {
// the param binding is a simple parameter
// e.g. (function a(a) {})
return false;
}

return name;
}
@@ -0,0 +1 @@
(function a([a, _a]) { a + _a })
@@ -0,0 +1,3 @@
(function a([_a2, _a]) {
_a2 + _a;
});
@@ -0,0 +1,3 @@
(function a([a]) { a });
(function a({ ...a }) { a });
(function a({ a }) { a });
@@ -0,0 +1,14 @@
(function a([_a]) {
_a;
});

(function a({ ..._a2
}) {
_a2;
});

(function a({
a: _a3
}) {
_a3;
});
@@ -0,0 +1,3 @@
{
"plugins": ["bugfix-safari-id-destructuring-collision-in-function-expression"]
}
@@ -0,0 +1,3 @@
import runner from "@babel/helper-plugin-test-runner";

runner(import.meta.url);
@@ -0,0 +1,56 @@
import { parseSync, traverse } from "@babel/core";
import { shouldTransform } from "../src/util.ts";

function getPath(input, parserOpts = {}) {
let targetPath;
traverse(
parseSync(input, {
parserOpts,
filename: "example.js",
configFile: false,
}),
{
FunctionExpression(path) {
targetPath = path;
path.stop();
},
},
);
return targetPath;
}

describe("shouldTransform", () => {
const positiveCases = [
"(function a([a]) {})",
"({ b: function a([a]) {} })",
"(function a({a}) {})",
"(function a(...a) {})",
"(function a({ ...a }) {})",
"(function a([a = 1]) {})",
"(function a(b, { a: [,...a] }) {})",
];

const negativeCases = [
"(function () {})",
"(function a() {})",
"(function a(a) {})",
"(function a() { var a })",
"(function b([a]) { var a })",
"(function b([a]) { function a() {} })",
"(function a(x = a) {})",
"(function a() { var { a } = {}; })",
"(function b([a]) { var { a } = {}; })",
"(function a({ [a]: b }) {})",
];

describe("the following cases should be transformed", () => {
test.each(positiveCases)("%p", input => {
expect(shouldTransform(getPath(input))).toBe("a");
});
});
describe("the following cases should not be transformed", () => {
test.each(negativeCases)("%p", input => {
expect(shouldTransform(getPath(input))).toBe(false);
});
});
});

0 comments on commit 29f697c

Please sign in to comment.