Skip to content

Commit

Permalink
breaking: remove useSpread and useBuiltIns option (#11141)
Browse files Browse the repository at this point in the history
* breaking: remove useSpread and useBuiltIns option

* chore: update test fixtures

* Make CRA e2e test pass

* Bump dependencies _before_ running "yarn install"

* chore: update test fixtures

* address review comments

* fix: assert.rejects should passed by async function

* Update packages/babel-plugin-transform-react-jsx/src/transform-classic.js

Co-authored-by: Brian Ng <bng412@gmail.com>

* update test fixtures

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>
Co-authored-by: Brian Ng <bng412@gmail.com>
  • Loading branch information
3 people committed Jun 25, 2020
1 parent 6e46372 commit 8cc8696
Show file tree
Hide file tree
Showing 30 changed files with 109 additions and 120 deletions.
81 changes: 4 additions & 77 deletions packages/babel-helper-builder-react-jsx/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,7 @@ You can set \`throwIfNamespace: false\` to bypass this warning.`,

let attribs = openingPath.node.attributes;
if (attribs.length) {
attribs = buildOpeningElementAttributes(attribs, file);
attribs = buildOpeningElementAttributes(attribs);
} else {
attribs = t.nullLiteral();
}
Expand All @@ -161,82 +161,9 @@ You can set \`throwIfNamespace: false\` to bypass this warning.`,
return call;
}

function pushProps(_props, objs) {
if (!_props.length) return _props;

objs.push(t.objectExpression(_props));
return [];
}

/**
* The logic for this is quite terse. It's because we need to
* support spread elements. We loop over all attributes,
* breaking on spreads, we then push a new object containing
* all prior attributes to an array for later processing.
*/

function buildOpeningElementAttributes(attribs, file) {
let _props = [];
const objs = [];

const { useSpread = false } = file.opts;
if (typeof useSpread !== "boolean") {
throw new Error(
"transform-react-jsx currently only accepts a boolean option for " +
"useSpread (defaults to false)",
);
}

const useBuiltIns = file.opts.useBuiltIns || false;
if (typeof useBuiltIns !== "boolean") {
throw new Error(
"transform-react-jsx currently only accepts a boolean option for " +
"useBuiltIns (defaults to false)",
);
}

if (useSpread && useBuiltIns) {
throw new Error(
"transform-react-jsx currently only accepts useBuiltIns or useSpread " +
"but not both",
);
}

if (useSpread) {
const props = attribs.map(convertAttribute);
return t.objectExpression(props);
}

while (attribs.length) {
const prop = attribs.shift();
if (t.isJSXSpreadAttribute(prop)) {
_props = pushProps(_props, objs);
objs.push(prop.argument);
} else {
_props.push(convertAttribute(prop));
}
}

pushProps(_props, objs);

if (objs.length === 1) {
// only one object
attribs = objs[0];
} else {
// looks like we have multiple objects
if (!t.isObjectExpression(objs[0])) {
objs.unshift(t.objectExpression([]));
}

const helper = useBuiltIns
? t.memberExpression(t.identifier("Object"), t.identifier("assign"))
: file.addHelper("extends");

// spread it
attribs = t.callExpression(helper, objs);
}

return attribs;
function buildOpeningElementAttributes(attribs) {
const props = attribs.map(attrib => convertAttribute(attrib));
return t.objectExpression(props);
}

function buildFragmentCall(path, file) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -377,6 +377,10 @@ export default function (
run(task);
}

async function runTaskAsync() {
run(task);
}

defaults(task.options, {
sourceMap: !!(task.sourceMappings || task.sourceMap),
});
Expand All @@ -399,7 +403,7 @@ export default function (
} else if (rejectMsg) {
delete task.options.rejects;

return assert.rejects(runTask, function (err) {
return assert.rejects(runTaskAsync, function (err) {
assert.ok(err.message.includes(rejectMsg));
return true;
});
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
/*#__PURE__*/
React.createElement(Foo, bar);
React.createElement(Foo, { ...bar
});
21 changes: 21 additions & 0 deletions packages/babel-plugin-transform-react-jsx/src/transform-classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,27 @@ export default declare((api, options) => {
const JSX_ANNOTATION_REGEX = /\*?\s*@jsx\s+([^\s]+)/;
const JSX_FRAG_ANNOTATION_REGEX = /\*?\s*@jsxFrag\s+([^\s]+)/;

if ("useSpread" in options) {
throw new Error(
'transform-react-jsx: Since Babel 8, an inline object with spread elements is always used, and the "useSpread" option is no longer available. Please remove it from your config.',
);
}

if ("useBuiltIns" in options) {
const useBuiltInsFormatted = JSON.stringify(options.useBuiltIns);
throw new Error(
`transform-react-jsx: Since "useBuiltIns" is removed in Babel 8, you must remove it from your config.
- Babel 8 now transforms JSX spread to object spread. If you need to transpile object spread with
\`useBuiltIns: ${useBuiltInsFormatted}\`, you can use the following config
{
"plugins": [
"@babel/plugin-transform-react-jsx",
["@babel/plugin-proposal-object-rest-spread", { "loose": true, "useBuiltIns": ${useBuiltInsFormatted} }]
]
}`,
);
}

// returns a closure that returns an identifier or memberExpression node
// based on the given id
const createIdentifierParser = (id: string) => () => {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/*#__PURE__*/
React.createElement(Component, babelHelpers.extends({}, props, {
React.createElement(Component, { ...props,
sound: "moo"
}));
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
/*#__PURE__*/
React.createElement(Component, babelHelpers.extends({}, x, {
React.createElement(Component, { ...x,
y: 2,
z: true
}));
});
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/*#__PURE__*/
React.createElement(Component, babelHelpers.extends({
React.createElement(Component, {
y: 2,
z: true
}, x));
z: true,
...x
});
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
/*#__PURE__*/
React.createElement(Component, babelHelpers.extends({
y: 2
}, x, {
React.createElement(Component, {
y: 2,
...x,
z: true
}));
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
var div = /*#__PURE__*/React.createElement(Component, babelHelpers.extends({}, props, {
foo: "bar"
}));
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"plugins": [["transform-react-jsx", { "useBuiltIns": false }]],
"rejects": "transform-react-jsx: Since \"useBuiltIns\" is removed in Babel 8, you must remove it from your config.\n- Babel 8 now transforms JSX spread to object spread. If you need to transpile object spread with\n`useBuiltIns: false`, you can use the following config\n{\n \"plugins\": [\n \"@babel/plugin-transform-react-jsx\",\n [\"@babel/plugin-proposal-object-rest-spread\", { \"loose\": true, \"useBuiltIns\": false }]\n ]\n}"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"plugins": ["transform-react-jsx", ["proposal-object-rest-spread", {
"loose": true,
"useBuiltIns": false
}], "external-helpers"]
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"plugins": [["transform-react-jsx", { "useBuiltIns": true }]],
"rejects": "transform-react-jsx: Since \"useBuiltIns\" is removed in Babel 8, you must remove it from your config.\n- Babel 8 now transforms JSX spread to object spread. If you need to transpile object spread with\n`useBuiltIns: true`, you can use the following config\n{\n \"plugins\": [\n \"@babel/plugin-transform-react-jsx\",\n [\"@babel/plugin-proposal-object-rest-spread\", { \"loose\": true, \"useBuiltIns\": true }]\n ]\n}"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"plugins": [["transform-react-jsx", { "useSpread": false }]],
"rejects": "transform-react-jsx: Since Babel 8, an inline object with spread elements is always used, and the \"useSpread\" option is no longer available. Please remove it from your config."
}
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
var div = <Component {...props} foo="bar" />
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"plugins": [["transform-react-jsx", { "useSpread": true }]],
"rejects": "transform-react-jsx: Since Babel 8, an inline object with spread elements is always used, and the \"useSpread\" option is no longer available. Please remove it from your config."
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
{
"plugins": ["transform-react-jsx", ["proposal-object-rest-spread", {
"loose": true,
"useBuiltIns": true
}]]
}

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

This file was deleted.

25 changes: 21 additions & 4 deletions packages/babel-preset-react/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ export default declare((api, opts) => {
const {
pure,
throwIfNamespace = true,
useSpread,
runtime = "classic",
importSource,
} = opts;
Expand All @@ -27,7 +26,27 @@ export default declare((api, opts) => {

// TODO: (Babel 8) Don't cast these options but validate it
const development = !!opts.development;
const useBuiltIns = !!opts.useBuiltIns;

if ("useSpread" in opts) {
throw new Error(
'@babel/preset-react: Since Babel 8, an inline object with spread elements is always used, and the "useSpread" option is no longer available. Please remove it from your config.',
);
}

if ("useBuiltIns" in opts) {
const useBuiltInsFormatted = JSON.stringify(opts.useBuiltIns);
throw new Error(
`@babel/preset-react: Since "useBuiltIns" is removed in Babel 8, you can remove it from the config.
- Babel 8 now transforms JSX spread to object spread. If you need to transpile object spread with
\`useBuiltIns: ${useBuiltInsFormatted}\`, you can use the following config
{
"plugins": [
["@babel/plugin-proposal-object-rest-spread", { "loose": true, "useBuiltIns": ${useBuiltInsFormatted} }]
],
"presets": ["@babel/preset-react"]
}`,
);
}

if (typeof development !== "boolean") {
throw new Error(
Expand All @@ -50,8 +69,6 @@ export default declare((api, opts) => {
pragmaFrag,
runtime,
throwIfNamespace,
useBuiltIns,
useSpread,
pure,
},
],
Expand Down
15 changes: 12 additions & 3 deletions scripts/integration-tests/e2e-create-react-app.sh
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,23 @@ cd tmp/create-react-app || exit
# TEST #
#==============================================================================#

# !!! WARNING !!!
# create-react-app uses the useBuiltIns: true option of @babel/preset-react,
# removed in Babel 8.0.0. Comment it out until they upgrade their Babel version.
sed -i 's/useBuiltIns: true/\/\/ useBuiltIns: true/' packages/babel-preset-react-app/create.js

bump_deps="$PWD/../../utils/bump-babel-dependencies.js"
node "$bump_deps"
for d in ./packages/*/
do
(cd "$d"; node "$bump_deps")
done

# Don't use Yarn 2
export YARN_IGNORE_PATH=1

startLocalRegistry "$PWD"/../../verdaccio-config.yml
yarn install
node "$PWD"/../../utils/bump-babel-dependencies.js
yarn lerna exec -- node "$PWD"/../../utils/bump-babel-dependencies.js
yarn install

# Test
CI=true yarn test
Expand Down

0 comments on commit 8cc8696

Please sign in to comment.