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

[RFC] Array spread syntax #6546

Closed
zth opened this issue Jan 2, 2024 · 4 comments
Closed

[RFC] Array spread syntax #6546

zth opened this issue Jan 2, 2024 · 4 comments
Labels
Milestone

Comments

@zth
Copy link
Collaborator

zth commented Jan 2, 2024

Array spreads are quite common in JS land, and they're pretty awkward to do right now in ReScript. You essentially need to do this (using Core):

let arr1 = [1, 2]
let arr2 = [3, 4]
let arr3 = [5, 6]

let arr = Array.concatMany([7, 8], [arr1, arr2, arr3])
let arrOnlySpreads = Array.concatMany([], [arr1, arr2, arr3])

Which compiles to:

var arr1 = [
  1,
  2
];

var arr2 = [
  3,
  4
];

var arr3 = [
  5,
  6
];

var arr = [
    7,
    8
  ].concat(arr1, arr2, arr3);

var arrOnlySpreads = [].concat(arr1, arr2, arr3);

Compiling to concat is fine imo, but it'd be great if the ReScript above could be written like this instead:

let arr1 = [1, 2]
let arr2 = [3, 4]
let arr3 = [5, 6]

let arr = [7, 8, ...arr1, ...arr2, ...arr3]
let arrOnlySpreads = [...arr1, ...arr2, ...arr3]

This could parse to the first ReScript snippet using concatMany, that's fine.

I believe this would be a pretty harmless change (as of now you need to write the exact code it parses to manually) and being a significant gain in ergonomics. Remains to be seen how difficult it is technically (need to account for things like [1, ...arr1, 2, 3,4 ...arr2] etc).

Thoughts?

@TheSpyder
Copy link
Contributor

concat seems to be pretty slow compared to the native array spread

Don’t trust micro benchmarks. The performance at scales of “two numbers per array” is so fast either way that it’s irrelevant. With larger arrays spread will naturally be slower because it’s iterating, not just copying.

One example I found without much research:
https://jonlinnell.co.uk/articles/spread-operator-performance

@zth
Copy link
Collaborator Author

zth commented Jan 2, 2024

@TheSpyder that's great! 😄 Sounds like that's not something worth worrying about then.

@cometkim
Copy link
Contributor

cometkim commented Jan 3, 2024

As the list{} syntax already supports this, it's good to have for all the first-class data literals we have, including #6545 and #6181

@zth zth added this to the v12 milestone Jan 9, 2024
@zth zth added the Syntax label Feb 2, 2024
@zth zth closed this as completed Feb 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants