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

Rest/spread placeholder #1

Closed
haltcase opened this issue Nov 29, 2017 · 8 comments
Closed

Rest/spread placeholder #1

haltcase opened this issue Nov 29, 2017 · 8 comments
Labels
breaking Breaking changes will occur as part of closing the issue. discussion [RFC] Fixes, APIs, or changes need feedback. type: feature Request for a new feature or enhancement.
Milestone

Comments

@haltcase
Copy link
Owner

prior art

These are both roughly equivalent to:

const log = (..._arg) => console.log('hi', ..._arg)

param.macro

input:

import { _ } from 'param.macro'
const log = console.log(..._)

current output:

const log = (_arg) => {
  return console.log(..._arg);
};

This ends up having to be called with an array (or other spreadable object) to work, ie. log([1, 2, 3]). Otherwise it throws an error.

proposed output:

const log = (..._arg) => {
  return console.log(..._arg);
};

This would be a breaking change at this point so it'd have to drop in v2.0.0 - but probably should happen.

@haltcase haltcase added breaking Breaking changes will occur as part of closing the issue. discussion [RFC] Fixes, APIs, or changes need feedback. type: feature Request for a new feature or enhancement. labels Nov 29, 2017
@haltcase haltcase added this to the v2.0.0 milestone Dec 3, 2017
@haltcase
Copy link
Owner Author

Closed by 8a47350.

@haltcase haltcase added status: pending release Issue is resolved but waiting to be released. and removed status: pending release Issue is resolved but waiting to be released. labels Dec 26, 2017
@rajington
Copy link

rajington commented Jun 22, 2019

Hey @citycide, great work on this library.

I'm curious with this pattern how to intentionally spread the first argument.

A good use case would be partially applying React components:

const Header = ({ title, subtitle}) => (
  <>
    <h1>{title}</h1>
    <h2></h2>
  <>
)

const CheckoutHeader = Header({ title: 'Checkout', ..._})

@haltcase
Copy link
Owner Author

@rajington hey 👋 , thanks

This would very likely require a new symbol specifically for this "asymmetric" rest/spread parameter. I've got a high bar for introducing additional symbols — I was reluctant to add lift but it's useful in several scenarios. This one would be more single-purpose.

One of my main goals is also to maintain a balance of sugar and clarity. And right now that balance is arguably as good it gets by doing these:

const CheckoutHeader = props => Header({ title: 'Checkout', ...props })
const CheckoutHeader = Header({ title: 'Checkout', subtitle: _ })

As always you're welcome to make a strong case in favor of it.

@rajington
Copy link

rajington commented Jun 23, 2019

Thanks for the quick reply, I feel there's some ambiguity that I misinterpreted and I could see how ..._ is the better of the two.

Could ...(_) work as a way to intentionally spread just the first param? Parens are already used to change intention of their contents (e.g. () => {} vs () => ({})) and maybe adding syntax with an example of spreading the first param could make the distinction more clear while still supporting both.

@haltcase
Copy link
Owner Author

@rajington it would be possible — Babel adds an extra object to the AST node with a parenthesized property. I'm not opposed to using it but is it really obvious that parenthesizing the spread placeholder would alter it that way?

@rajington
Copy link

rajington commented Jun 24, 2019

Yeah, hard to say ...(_) would be obvious without showing usages/implications of both, but potentially something that people could learn once and remember easily, and likely no one will be wrapping it needlessly and getting unexpected results.

Also maybe it's not that big of a deal to not have a way to spread the first prop, I'm worried others might combine them in the way that I had but can definitely wait for it to come up.

Maybe I should move this to the TC39 proposal? Not having the bare ... and adding the parens seems to account for all cases:

macro y = transpiled y = y(a,b) calls f with
f(_) (x) => f(x) a
f(..._) (...x) => f(...x) a, b
f([..._]) (...x) => f([...x]) [a, b]
f({..._}) (...x) => f({...x}) {0: a, 1: b}
f([...(_)]) (x) => f([...x]) [...a]
f({...(_)}) (x) => f({...x}) {...a}

@haltcase
Copy link
Owner Author

@rajington there was already some discussion on the proposal over here, which is in part why the ... rest/spread syntax has been removed from the initial proposal and saved for later.

Feel free to introduce them to the idea. I'd be much more open to implementing that if it were backed up by an upcoming proposal.

@rajington
Copy link

That makes a lot of sense, thank you so much for your help. This is such an exciting area!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking changes will occur as part of closing the issue. discussion [RFC] Fixes, APIs, or changes need feedback. type: feature Request for a new feature or enhancement.
Projects
None yet
Development

No branches or pull requests

2 participants