Skip to content
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

Object rest spread not working correctly in object destructuring (T7086) #4074

Closed
babel-bot opened this issue Feb 9, 2016 · 23 comments

Comments

Projects
None yet
8 participants
@babel-bot
Copy link
Collaborator

commented Feb 9, 2016

Issue originally made by @xinchaobeta

Bug information

  • Babel version: 6.4.5
  • Node version: v4.1.0
  • npm version: 2.14.3

Options

{
  "plugins": ["transform-object-rest-spread"]
}

Input code

let {a, b, ...c} = d

Description

the output code is

let { a, b, ...c } = d;

we expect

let {a, b} = d
let c = _objectWithoutProperties(d, ["a", "b"]);

In the above example, we have not include the es2015-destructuring , because we only want compile es7 to es6

@babel-bot

This comment has been minimized.

Copy link
Collaborator Author

commented Feb 9, 2016

Comment originally made by @hzoo

You don't need to assign anyone.

Yeah the transform only visits ObjectExpression

@babel-bot

This comment has been minimized.

Copy link
Collaborator Author

commented Apr 1, 2016

Comment originally made by @dlmanning

Is there a lot involved in making this work apart from es2015-destructuring? Node 6.0 will support destructuring natively when it's released this month, seems a shame to have to keep transpiling destructuring in order to get object-rest-spread.

@babel-bot

This comment has been minimized.

Copy link
Collaborator Author

commented May 4, 2016

Comment originally made by @taion

I'm hitting this issue now with attempting to transpile code for Node 6 – I can't apply just the object rest spread transform without also including destructuring.

@babel-bot

This comment has been minimized.

Copy link
Collaborator Author

commented May 4, 2016

Comment originally made by @loganfsmyth

The short-term fix for this is to enable the destructuring transform, even on Node 6. We'll have to work out a fix, but it won't be right away unfortunately.

@babel-bot

This comment has been minimized.

Copy link
Collaborator Author

commented May 4, 2016

Comment originally made by @taion

One more note – transform-es2015-parameters is required as well to handle destructuring in function args.

@babel-bot

This comment has been minimized.

Copy link
Collaborator Author

commented Aug 8, 2016

Comment originally made by @morlay

Just Note:

babel-plugin-transform-es2015-spread, babel-plugin-transform-es2015-parameters and babel-plugin-transform-es2015-destructuring are required, if we write code like below:

const fn = ({ a, ...otherProps }) => otherProps;

fn({ a: 1, b: 2 });

Tested in Node 6 with babel@6

@olalonde

This comment has been minimized.

Copy link

commented Sep 12, 2016

Is this issue still active?

@hzoo

This comment has been minimized.

Copy link
Member

commented Sep 12, 2016

Yep! all the open issues would be active, just no one with a PR yet

@hzoo hzoo added the i: bug label Sep 12, 2016

@olalonde

This comment has been minimized.

Copy link

commented Sep 12, 2016

Ok, would need this for https://github.com/blockai/babel-preset-eslatest-node6. No time for a PR though :(

@motiz88

This comment has been minimized.

Copy link
Contributor

commented Sep 12, 2016

I may have some time to put together a PR for this. What would be the preferred way forward?

@danez

This comment has been minimized.

Copy link
Member

commented Sep 12, 2016

I think the preferred outcome should be that the transform-object-rest-spread should support destructuring without having to enable the other plugins. Though I don't know yet how to get there or what the current problem is.

@olalonde

This comment has been minimized.

Copy link

commented Sep 12, 2016

I believe it also depends on transform-es2015-parameters for spread in function parameters.

@hzoo hzoo added the help wanted label Sep 13, 2016

@christophehurpeau

This comment has been minimized.

Copy link
Contributor

commented Oct 11, 2016

@hzoo I can try to help. Should we extract/use visitors like parameters destructuring or class DestructuringTransformer ; or build another system ?

@hzoo

This comment has been minimized.

Copy link
Member

commented Oct 13, 2016

We could extract it but it depends on the output like mentioned above.

// input
let { a, b, ...c } = d;
// possible output
let {a, b} = d
let c = _objectWithoutProperties(d, ["a", "b"]);
// input
function foo({ x, y, ...z }) {}

// current output with destructuring-transform
function foo(_ref) {
  var x = _ref.x;
  var y = _ref.y;
  var z = _objectWithoutProperties(_ref, ["x", "y"]);
}

// possible output
function foo(_ref) {
  var { x, y } = _ref;
  var z = _objectWithoutProperties(_ref, ["x", "y"]);
}
@Jessidhia

This comment has been minimized.

Copy link
Member

commented Oct 14, 2016

I had started working on something like this in transform-object-rest-spread, but I no longer have the code; forgot to backup the babel folder when I wiped my system...

Anyway, I had gotten what @hzoo mentioned to work, but then got stuck with having to support nested destructuring, especially object destructuring nested in array destructuring.

@ashwell

This comment has been minimized.

Copy link

commented Oct 17, 2016

Hello kind babel wizards,

I was just adding 'babel-plugin-transform-object-rest-spread' to a webpack config that was already using 'babel-preset-es2015'. The object rest/spread transform wasn't working until I disabled 'babel-plugin-transform-runtime'. Maybe that is a known to you guys but I thought I'd add a note here anyway.

Thank you for making such a wonderful tool that grants me the ability to live in the future.

@hzoo

This comment has been minimized.

Copy link
Member

commented Oct 19, 2016

Since is this high priority + (given I'm working on https://github.com/babel/babel-preset-env and having to whitelist the destructuring/parameter transforms if using object spread sucks), I'm going to work on this.

@hzoo hzoo self-assigned this Oct 19, 2016

@hzoo

This comment has been minimized.

Copy link
Member

commented Oct 19, 2016

PR #4755

@hzoo

This comment has been minimized.

Copy link
Member

commented Nov 14, 2016

Ok the PR is #4755

Can folks try the changes locally and report back?

Can test locally by replacing in node_modules: https://gist.github.com/hzoo/d269ccfd5758c556c900f47dd752cd61 - I just took the code that changed in src and ran it in the babel repl in loose mode

replace node_modules/babel-plugin-transform-object-rest-spread/lib/index.js

@hzoo

This comment has been minimized.

Copy link
Member

commented Nov 14, 2016

If no one can test what oss repos are using node 6 and want to use object-spread standalone?

@olalonde

This comment has been minimized.

Copy link

commented Nov 14, 2016

@hzoo I'd like to test but not sure what to do. Here's the babel preset I use for most of my projects: https://github.com/blockai/babel-preset-blockai/blob/master/index.js. I'd like to get rid of transform-es2015-destructuring and transform-es2015-parameters. Should I remove those plugins and do the copy/paste to see if anything breaks?

@hzoo

This comment has been minimized.

Copy link
Member

commented Nov 14, 2016

Yeah I would remove the 2 plugins + replace node_modules/babel-plugin-transform-object-rest-spread/lib/index.js

@hzoo hzoo closed this in #4755 Nov 15, 2016

@olalonde

This comment has been minimized.

Copy link

commented Nov 28, 2016

@hzoo tried to bump babel-plugin-transform-object-rest-spread to 6.19.0 and get rid of babel-plugin-transform-es2015-parameters / babel-plugin-transform-es2015-destructuring in https://github.com/blockai/babel-preset-eslatest-node6/pull/1/files

My test fails with:

/Users/olalonde/code/blockai/babel-preset-eslatest-node6/test/object-rest-spread.js:28
  const fn = ({ email, ...rest } = {}) => {
                       ^^^
SyntaxError: Unexpected token ...
    at Object.exports.runInThisContext (vm.js:76:16)
    at Module._compile (module.js:528:28)
    at loader (/Users/olalonde/code/blockai/babel-preset-eslatest-node6/node_modules/babel-register/lib/node.js:144:5)
    at Object.require.extensions.(anonymous function) [as .js] (/Users/olalonde/code/blockai/babel-preset-eslatest-node6/node_modules/babel-register/lib/node.js:154:7)
    at Module.load (module.js:473:32)
    at tryModuleLoad (module.js:432:12)
    at Function.Module._load (module.js:424:3)
    at Module.require (module.js:483:17)
    at require (internal/module.js:20:19)
    at Object.<anonymous> (/Users/olalonde/code/blockai/babel-preset-eslatest-node6/test/object-rest-spread.test.js:7:1)

Is that expected?

edit: already reported here: #4851

rgbkrk added a commit to rgbkrk/babel-preset-es2015-node6 that referenced this issue Dec 22, 2016

object rest spread now well supported
babel/babel#4074 has been handled in `babel-plugin-transform-object-rest-spread`. As of https://github.com/babel/babel/releases/tag/v6.19.0 `babel-plugin-transform-object-rest-spread` now works well with this preset.

jhen0409 added a commit to jhen0409/babel-preset-es2015-node6 that referenced this issue Dec 24, 2016

object rest spread now well supported (#9)
babel/babel#4074 has been handled in `babel-plugin-transform-object-rest-spread`. As of https://github.com/babel/babel/releases/tag/v6.19.0 `babel-plugin-transform-object-rest-spread` now works well with this preset.

@lock lock bot added the outdated label May 6, 2018

@lock lock bot locked as resolved and limited conversation to collaborators May 6, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
You can’t perform that action at this time.