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

JSX Proposal: ES6-like property value shorthand #118

Closed
wants to merge 3 commits into from
Closed

JSX Proposal: ES6-like property value shorthand #118

wants to merge 3 commits into from

Conversation

nojvek
Copy link

@nojvek nojvek commented Jun 9, 2019

This discussion has happened in several repos now.

microsoft/TypeScript#31797
#23
facebook/react#2536

Currently the JSX syntax supports the following transformations. Using h for brevity.

<div title="hello" />
 h(`div`, {title: 'hello'}) >

const title = 'foo';
<div title={title} />

^ seems verbose

h(`div`, {title}) >

<input checked />

Attribute on it's own always resolves to true (inspired from html)

h(`input`, {checked: true}) >

const props = {a: 1, b: 2, c: 3};
<Foo {...props} />

JSX spread operator is awesome 🎉. It came before object spread and shorthand was finalized in ES spec. So we can understand why {....x} is strict. We can <div {...{title}} /> though, which is ugly.

h(Foo, {...props}) >

New Proposal: Use {Identifier} shorthand which would be a superset of JSXSpreadAttribute.

Object shorthand and spread is now supported in every major js engine and browsers. It's common js lingo now.

const d = 4;
const props = {a: 1, b: 2, c: 3};
<input checked {...props, d} />
h(`input`, {checked: true, ...props, d} >

const x = 1;
const y = 2;
<div {x} {y} />
h(`div`, {x, y}) >

The kind proposal is to upgrade JSX attribute spread syntax to also include {x} shorthand syntax that have been part of ES Spec and implemented in all major engines. It's a non breaking change since JSXAttributeSpread remains as is.

The transform is simply merging multiple object expressions into a single object expression and send it as second argument to jsxFactory(component: string | Component, attrs: {[attr]: any}, ...children). For a down transpile to ES5, use Object.assign as usual.

This would make writing JSX more expressive with less typing to do. The learning curve is minimal too since it's piggy-backing on the powerful ES object expression syntax.

As from the react documentation, they don't recommend using the normal attribute shorthand https://reactjs.org/docs/jsx-in-depth.html#props-default-to-true

In their words In general, we don’t recommend using this because it can be confused with the ES6 object shorthand {foo} which is short for {foo: foo} rather than {foo: true}. This behavior is just there so that it matches the behavior of HTML.

This would remove some of that confusion and give an elegant ES6-like way to move foward without complicating jsx spec.

I'm hoping the 👨‍🚀 👩‍🚀 contributors and community are at-least open to the idea.

@nojvek nojvek changed the title JSX: ObjectExpression to specify attributes which is a superset of object spread JSX Proposal: ObjectExpression to specify attributes which is a superset of object spread Jun 9, 2019
@nojvek
Copy link
Author

nojvek commented Jun 13, 2019

@sebmarkbage could this get some 👀 please.

If this gets accepted as jsx syntax, happy to make a Babel and typescript proof of concept PRs

@sebmarkbage
Copy link
Contributor

This has an unfortunate lack of parity with how the same syntax works in the position of children. E.g. <div>{...children}</div> doesn't necessarily follow this same rule. The brackets in that case doesn't mean "object literal". It means something more like "escape out of the template and put some raw JS in this value position".

The concerns raised in other threads such as the confusion between <div {checked} /> and <div checked /> remains.

Do you foresee any other possible problems with syntax extensions to object literals? E.g. immutable objects like #{ foo } or {| foo |}? Or possibly decorators on objects or object methods?

When multiple object literals are used, is it confusing that they're actually effectively part of one larger object initializer? With new syntax like immutable objects, there would be no way of resolving conflicting definitions.

I'm trying to think through other possible additions we might like to do that would conflict with this approach.

I don't think I'd want this to be the solution for a JSX 2.0 world where we could make more breaking changes to avoid the superfluous punctuation. However the bar for doing something like JSX 2.0 is very high and needs to be much more carefully considered. That said, this is probably the least objectionable non-breaking change that we could reasonably easily do.

I'm curious to hear @ljharb's thoughts on this extension.

@ljharb
Copy link

ljharb commented Jun 13, 2019

Given that <Foo {...props} /> works, it does seem reasonable to allow <Foo {...props, a, b} /> and <Foo {a, b} />. I think the confusion is a legitimate concern, but the thing that seems more confusing to me is the implicit ={true} (which is nice because it's html-like) than the "only partial" support of object syntax.

Perhaps if the spread needed to be outside the curlies? ie, if <Foo ...props /> was required, then <Foo ...{a, b} /> or <Foo ...{...props} /> would make sense to me.

@sebmarkbage
Copy link
Contributor

Perhaps if the spread needed to be outside the curlies? ie, if <Foo ...props /> was required, then <Foo ...{a, b} /> or <Foo ...{...props} /> would make sense to me.

Originally, one rationale against this was that this syntax wouldn't be possible in the children position.

E.g. neither <Foo>...children</Foo> nor <Foo>...{children}</Foo>

@ljharb
Copy link

ljharb commented Jun 14, 2019

That's a good point, but it seems pretty odd to pass an object as children (it's weird to me whenever children isn't a node, otherwise why use the children position)

@nojvek
Copy link
Author

nojvek commented Jun 14, 2019

objectexpression sounds like dangerous territory since we don't know how object expression may evolve in future. Makes sense. How about we just support identifiers and spreads for the time being.

i.e <Foo {x} {y} /> or <Foo {...props, x, y} /> or <Foo {x, y, z} />.

Ideally It would be awesome if <Foo checked /> wouldn't have force resolved to {checked: true}, so we could do <Foo x y /> but that ship has sailed so this is a form of shorthand we should settle for.

I will update the PR and title.

@ljharb
Copy link

ljharb commented Jun 14, 2019

We could also allow a comma separated list of identifiers.

Separately, what about prop names that aren’t identifiers? Those can be quoted in the one off form.

@nojvek nojvek changed the title JSX Proposal: ObjectExpression to specify attributes which is a superset of object spread JSX Proposal: ES6-like property value shorthand Jul 19, 2019
@nojvek
Copy link
Author

nojvek commented Jul 19, 2019

Okay I've updated the PR title, description and the grammar.

My BNF skills are a bit rusty. I'm not sure what IdentifierStart and IdentifierPart means. Please help me make it nice and shiny.

This will allow <Foo {x} {y} /> or <Foo {...props, x, y} /> or <Foo {x, y, z} />.

Kept trailing commas as optional to be in sync with it's js counterpart.

only allowing identifiers at moment since const 'hello-world' = 'abc' is not supported in JS. i.e only identifiers work as shorthands in js anyway.

We can extend this to support assignment in future if we wanted but I want to minimize the scope so its a simple change that doesn't break anything and somewhat future proof.

@GnsP
Copy link

GnsP commented Sep 6, 2019

@nojvek , @ljharb , @sebmarkbage
I recently opened a PR on this very same topic.

However I am proposing a different (non conflicting) syntax for the shorthand. I propose writing shorthand attributes as <Component +shorthandProp1 +shorthandProp2 />. And to allow this syntax strictly for valid JS Identifiers (not JSX Names, which can be namespaced and can include a dash). As @nojvek pointed out, only JS identifiers work as shorthand attributes anyway.

I would like to know your thoughts on it.

@ljharb
Copy link

ljharb commented Sep 6, 2019

@GnsP that seems confusing - +identifier coerces it to a Number in JS.

@nojvek
Copy link
Author

nojvek commented Sep 13, 2019

@sebmarkbage I take it this PR most likely won't get accepted right? I usually like to keep my open PRs list clean since I tend to send a lot of open source PRs to multiple repos.

I get a feeling JSX is eternally stuck in it's state which is both good and bad.

If you think chances are low, I'd rather close it.

@nojvek
Copy link
Author

nojvek commented Sep 26, 2019

I'm closing this. From what I see JSX is stuck in analysis paralysis state forever and unless we get some sort of a committee from (babel, react, typescript) teams trying to drive it forward, the discussions here aren't going anywhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants