-
-
Notifications
You must be signed in to change notification settings - Fork 26.9k
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
Enable useBuiltIns option on object-rest-spread #902
Conversation
@@ -14,7 +14,10 @@ const plugins = [ | |||
// class { handleClick = () => { } } | |||
require.resolve('babel-plugin-transform-class-properties'), | |||
// { ...todo, completed: true } | |||
require.resolve('babel-plugin-transform-object-rest-spread'), | |||
[require.resolve('babel-plugin-transform-object-rest-spread'), { | |||
// Use Object.assign directly, instead of extends helper |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's add a note that this requires Object.assign
polyfill, and do it in README of the preset too.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
af794af
to
29df885
Compare
Based on #399 it looks like it's already being polyfilled so this wouldn't be a breaking change? |
It's polyfilled for Create React App users, but not for people who use standalone plugin. |
Sidenote, in a separate PR we can also enable |
@existentialism Want to do it here as well? |
@gaearon sure! |
29df885
to
e73f3f0
Compare
I tried using spread locally on your branch and I still see the |
Strange, I'm seeing the following snippet from render() {
const a = {
foo: 'bar'
};
const b = {
...a,
baz: 'bat'
};
return (
<div className="App" {...b}> Without PR: // Helper above:
var _extends = Object.assign || function (target) { for (var i = 1; i < arguments.length; i++) { var source = arguments[i]; for (var key in source) { if (Object.prototype.hasOwnProperty.call(source, key)) { target[key] = source[key]; } } } return target; };
_createClass(App, [{
key: 'render',
value: function render() {
var a = {
foo: 'bar'
};
var b = _extends({}, a, {
baz: 'bat'
});
return _react2.default.createElement(
'div',
_extends({ className: 'App' }, b, { With PR: // No helper above...
_createClass(App, [{
key: 'render',
value: function render() {
var a = {
foo: 'bar'
};
var b = Object.assign({}, a, {
baz: 'bat'
});
return _react2.default.createElement(
'div',
Object.assign({ className: 'App' }, b, { |
@fson Yarn is failing the build here, any ideas why? |
Weird, maybe a temporary glitch? |
Yea. Leaving this to you to review. |
Looks good to me! |
* Enable useBuiltIns option on object-rest-spread * note polyfill requirement * Enable useBuiltIns with transform-react-jsx * Add direct ref to transform-react-jsx
* Enable useBuiltIns option on object-rest-spread * note polyfill requirement * Enable useBuiltIns with transform-react-jsx * Add direct ref to transform-react-jsx
* Enable useBuiltIns option on object-rest-spread * note polyfill requirement * Enable useBuiltIns with transform-react-jsx * Add direct ref to transform-react-jsx
Addresses #899