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

Merged Rest and Spread operation (T6782) #3840

Open
babel-bot opened this issue Dec 7, 2015 · 10 comments · May be fixed by #7087 or #10215
Open

Merged Rest and Spread operation (T6782) #3840

babel-bot opened this issue Dec 7, 2015 · 10 comments · May be fixed by #7087 or #10215

Comments

@babel-bot
Copy link
Collaborator

Issue originally made by @sebmarkbage

Description

It is plausible that rest/spread operations can be optimized to be merged into a single operations if there are no side-effects between them. E.g. this:

let { a, b, ...rest } = obj;
let abc = { ...rest, c: 123 };

can be optimized to this:

let abc = combinedRestSpread(/*copied*/ obj, /*new object*/ { c: 123 }, /*excluded names*/ ['a', 'b']);
@Hrily
Copy link

Hrily commented Oct 31, 2017

Hie

I would like to contribute to this issue :)

It would be helpful if you could tell me how should I start?

@perinikhil
Copy link
Contributor

Hi guys, has this issue been claimed ? :)

@Andarist
Copy link
Member

@perinikhil doesn't seem so, if you want to work in this go ahead, you can join our slack if you need any help.

@simplymichael
Copy link

simplymichael commented Dec 20, 2017

Hi, I implemented a rather basic -but working- function for this, am I allowed to paste the function here?

@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Dec 20, 2017

@simplymichael If @perinikhil Started working on this issue you two can discuss it on Slack, otherwise you could setup a partial PR 😉

@Jessidhia
Copy link
Member

This seems to only be the case, in the example code, if a and b are dead (write-only) assignments.

Removing the first line in case it was determined to be used only as input for the following spread would also skip executing any getters (or Proxy traps) for a and b.

@perinikhil
Copy link
Contributor

@simplymichael
its really awesome to see the level of enthusiasm you have :)
unfortunately i havent got the chance to work on this issue yet as i was having internet problems over the past few days ( thankfully its all resolved now :D)

Lets catch up on slack and get this PR into shape 👍

@simplymichael
Copy link

@perinikhil
Thank you. That's really encouraging.
Okay, my slack display name is the same as on here: simplymichael.

@motiko
Copy link

motiko commented Sep 21, 2018

what is preventing this PR from being merged?
is there anything i can help with?

@Andarist
Copy link
Member

You could start a new competing PR (the other one is largely incomplete) 😉 If you need any help you can always reach to us on slack

@tanhauhau tanhauhau linked a pull request Jul 13, 2019 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
10 participants