Spread in objects #4473

zdenko · 2017-03-23T16:07:01Z

Connected to #3894.

In CS2 emit

obj = { x: 1, y: 2}
obj1 = {obj..., z: 3}

to

obj1 = {...obj, z: 3}

Improve destructuring assignment for object literal:

{a, b, x...} = {a:1, b:2, c:3, d:4, e:5}
a = 1
b = 2
x = {c:3, d:4, e:5}

{c, x..., e} = {a:1, b:2, c:3, d:4, e:5}
c = 3
e = 5
x = {a:1, b:2, d:4}

{c, x..., a:p} = {a:1, b:2, c:3, d:4, e:5}
c = 3
p = 1
x = {b:2, d:4, e:5}

futurists =
  sculptor: "Umberto Boccioni"
  painter:  "Vladimir Burliuk"
  poet:
    name:   "F.T. Marinetti"
    address: [
      "Via Roma 42R"
      "Bellagio, Italy 22021"
    ]

{poet: {name, addr1...}} = futurists
addr1 = {address: [ "Via Roma 42R",  "Bellagio, Italy 22021"]}

{poet: {addr2..., name:title}} = futurists
addr2 = {address: [ "Via Roma 42R",  "Bellagio, Italy 22021"]}

vendethiel · 2017-03-23T16:13:04Z

src/nodes.coffee

@@ -1748,6 +1748,13 @@ exports.Assign = class Assign extends Base
    vvarText = fragmentsToText vvar
    assigns  = []
    expandedIdx = false
+    # check if variable is object and containes splat, e.g. {a, b, c...}
+    hasSplat = isObject and (i for obj, i in objects when obj instanceof Splat).length == 1


I think it's better to error out if length>1?

Since multiple splats/expansions are disallowed in an assignment, length == 1 should be ok?

No. We need to show a nice error message, like when you try to use multiple splats in array desctructuring or parameter list.

This is the current status :

./bin/coffee -e "{a, x..., c, y...} = {a:1, b:2, c:3, d:4, e:5}" [stdin]:1:14: error: multiple splats/expansions are disallowed in an assignment {a, x..., c, y...} = {a:1, b:2, c:3, d:4, e:5}

lydell · 2017-03-23T16:57:19Z

test/assignment.coffee

+test "destructuring assignment with objects and splats: ES2015", ->
+  obj = {a:1, b:2, c:3, d:4, e:5}
+  {a:x, b, r...} = obj
+  {a:y, c..., d} = obj


The rest element has to be last, according to Babel: http://babeljs.io/repl/#?babili=false&evaluate=false&lineWrap=false&presets=es2015%2Cstage-2%2Cstage-3&targets=&browsers=&builtIns=false&code=const%20%7Ba%3Ay%2C%20...c%2C%20d%7D%20%3D%20obj&experimental=false&loose=false&spec=false&playground=true

I guess CoffeeScript should support it anywhere, though, just like for arrays? I guess we just need to move the splat last in the compiled JS.

lydell · 2017-03-23T16:58:14Z

test/assignment.coffee

+  eq c.b, b
+  eq r.e, c.e
+
+test "destructuring assignment with context (@) properties: ES2015", ->


I don't see any @s in this test?

That's a typo. I copied the string.

lydell · 2017-03-23T16:59:34Z

src/nodes.coffee

@@ -1748,6 +1748,13 @@ exports.Assign = class Assign extends Base
    vvarText = fragmentsToText vvar
    assigns  = []
    expandedIdx = false
+    # check if variable is object and containes splat, e.g. {a, b, c...}
+    hasSplat = isObject and (i for obj, i in objects when obj instanceof Splat).length == 1


No. We need to show a nice error message, like when you try to use multiple splats in array desctructuring or parameter list.

lydell · 2017-03-23T17:02:27Z

test/objects.coffee

+  # obj2 = "#{{obj1..., b: 3}}"
+  t1 = CoffeeScript.compile "{obj1..., b: 3}", bare: yes
+  t1 = t1.replace(/\s+/g, "")
+  eq t1, "({...obj1,b:3});"


The rule has been to never compare strings with the compiled output JS, unless there's no other way to assert that the compiled code works as intended. Also, CoffeeScript always strives to produce readable output. You need to add some spaces in there.

Error for multiple splats is already there at line 1791 in lib/nodes.coffee

Did you reply to the wrong comment? :)

Anyway, we need a test for the multiple splats error.

Sorry. Long hours :(

Anyway, I couldn't find example how to test the output of obj2 = {obj1..., b:5}
CS compiles it to

(function() { var obj2; obj2 = { ...obj1, b: 5 }; }).call(this);

but test fails since the syntax {...obj1, b:5} is currently in stage-3 and requires Babel.

To rephrase the question, can Babel be used for tests?

It is our policy to only support ES features that have reached stage 4, and are supported in the Node runtime (modules are the big exception to this). Therefore the test would compile to the finalized ES syntax, and Node should be able to run it.

GeoffreyBooth · 2017-03-23T17:50:46Z

@zdenko thank you for making this effort! Do you feel like helping with the destructuring branch? The goal there is to output ES2015 destructured syntax whenever possible. Certain Coffee destructuring arrangements aren’t possible in ES, so should stay as they are; and default values should follow the ES style of only getting applied with the variable is undefined, not undefined or null.

zdenko · 2017-03-23T23:27:27Z

@GeoffreyBooth sure.

So, the output of destructuring
{a, b, x...} = {a:1, b:2, c:3, d:4}
should be
{a, b, ...x} = {a:1, b:2, c:3, d:4}

And, the rest element has to be the last element.

GeoffreyBooth · 2017-03-24T03:09:42Z

@zdenko that syntax doesn’t seem to work for me, but in general, yes, whenever there is an ES destructured syntax that is equivalent to a CoffeeScript one, we want to output the ES one. In #4311 I refactored the handling of function parameters to support expansions and splats output as ES; if the splat was the last parameter, the conversion was straightforward, otherwise a placeholder splat became the last parameter and further assignments happened in the function body. We could do something similar for destructuring.

zdenko · 2017-03-24T06:08:11Z

@GeoffreyBooth this syntax should work.

The proposal for object rest/spread properties is in stage-3.

Since CS supports only stage-4, the output of object spread should be:
n = {x, y, z...} => n = Object.assign({}, x, y, z}
n = { a..., { x: 1, y: 2 }... } => n = Object.assign({}, a, { x: 1, y: 2 })

Is that correct?

GeoffreyBooth · 2017-04-09T05:24:53Z

Closing in favor of #4493

Zdenko Vujasinovic added 4 commits March 20, 2017 15:47

object literal spread

79da334

object spread

5ebeb55

object spread assign

3b6db94

object spread tests

e7e53dd

vendethiel reviewed Mar 23, 2017

View reviewed changes

fix splat condition to catch multiple splats/expansions

78774b1

zdenko force-pushed the objectSpread branch from 6ad726c to 78774b1 Compare March 23, 2017 16:47

lydell requested changes Mar 23, 2017

View reviewed changes

LinusBorg mentioned this pull request Apr 8, 2017

Coffee-/ plain JavaScript? vuejs/vuex#20

Closed

GeoffreyBooth mentioned this pull request Apr 9, 2017

[CS2] Destructuring object spreads #4493

Merged

GeoffreyBooth closed this Apr 9, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Spread in objects #4473

Spread in objects #4473

zdenko commented Mar 23, 2017 •

edited

Loading

vendethiel Mar 23, 2017

zdenko Mar 23, 2017

lydell Mar 23, 2017

zdenko Mar 23, 2017

lydell Mar 23, 2017

lydell Mar 23, 2017

zdenko Mar 23, 2017

lydell Mar 23, 2017

lydell Mar 23, 2017

zdenko Mar 23, 2017

lydell Mar 23, 2017

zdenko Mar 23, 2017

GeoffreyBooth Mar 24, 2017

GeoffreyBooth commented Mar 23, 2017

zdenko commented Mar 23, 2017 •

edited

Loading

GeoffreyBooth commented Mar 24, 2017 •

edited

Loading

zdenko commented Mar 24, 2017

GeoffreyBooth commented Apr 9, 2017

Spread in objects #4473

Spread in objects #4473

Conversation

zdenko commented Mar 23, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

GeoffreyBooth commented Mar 23, 2017

zdenko commented Mar 23, 2017 • edited Loading

GeoffreyBooth commented Mar 24, 2017 • edited Loading

zdenko commented Mar 24, 2017

GeoffreyBooth commented Apr 9, 2017

zdenko commented Mar 23, 2017 •

edited

Loading

zdenko commented Mar 23, 2017 •

edited

Loading

GeoffreyBooth commented Mar 24, 2017 •

edited

Loading