-
Notifications
You must be signed in to change notification settings - Fork 131
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: React Expressions with Implicit Do-Generator Semantics #98
Conversation
README.md
Outdated
- JSXElement | ||
- JSXFragment | ||
|
||
JSXGeneratorExpression : | ||
|
||
- [lookahead ∈ { `{` }] ObjectLiteral |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This checks for equality, which is unnecessary since ObjectLiteral
will require it, but it also leaves it ambiguous because it'll match both branches.
Instead, use a negative lookahead on the StatementList
below.
README.md
Outdated
@@ -95,7 +96,7 @@ JSXAttributes : | |||
|
|||
JSXSpreadAttribute : | |||
|
|||
- `{` `...` AssignmentExpression `}` | |||
- `{` `...` JSXGeneratorExpression `}` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't really work. You can't have a spread beginning a sequence of statements. I think for spread we'll need to leave it as AssignmentExpression
. (Although it is really weird that this can be a SequenceExpression
.)
Same thing in JSXChildExpression
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it was a bit odd to define JSXGeneratorExpression
as a StatementList
, but rather I should define it with the surround braces: { StatementList }
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think of this?
interface JSXExpressionContainer <: Node {
type: "JSXExpressionContainer";
expression: Expression | JSXEmptyExpression | JSXStatementList;
}
interface JSXStatementList <: Node {
type: "JSXStatementList";
body: [Statement];
generator: boolean;
}
AST.md
Outdated
type: "JSXExpressionContainer"; | ||
expression: Expression | JSXEmptyExpression; | ||
statements: [ Statement | Declaration ]; | ||
hasYield: boolean; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this a requirement? Seems like the transform could figure that out if needed. I suppose it makes transform order and such a bit easier to deal with but for Babel I think it should be fine to visit the tree on demand.
It's probably helpful to have though. We can just call it generator
then since arrow function has that flag. Later we could add async
as well if there's an await
.
AST.md
Outdated
type: "JSXExpressionContainer"; | ||
expression: Expression | JSXEmptyExpression; | ||
statements: [ Statement | Declaration ]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Declaration is a Statement in ESTree terminology (unlike the ES spec). Specifying both is superfluous.
We should probably go for backwards compatibility here. Lots of people can have transforms, linters and other things that depend on this.
We could have a flag that switches between the statement list form or expression form depending on if there are more than one expression. Similar to how ArrowFunctionExpression
works.
Ok, I noticed another problem with this change.
This makes this syntax errors:
Since you can't have unnamed function/class declarations. These two evaluate to
That's a pretty bad breaking change. We'll have to figure something out for these. We could do the same thing that we do for object literals and lookahead? They'll all have the same problem. They require over-parenthesization when used in the statement list form.
|
👍 But I would much prefer making do-expression vs generator explicit. PS. Would |
I agree with @syranide. If we can find a way to be more explicit, about that it is a We were discussing about introducing keywords with an asterisk behind (if*, for*, switch*, while*), in in this issue: sebmarkbage/ecmascript-generator-expression#1 (comment) It would then look like this: <div>{ for* (let item of items) yield item; }</div> |
@loganfsmyth brought up a good point which I'll quote here:
|
First, let me just state my perspective on the state of things. I think that JSX is not a given winner. There are alternative approaches like templates that can come up with whatever syntax to overcome obstacles like this. But also JavaScript as the source language isn't even a given winner for this since new forks can avoid existing syntactic quirks. If we don't stay on top of getting the best expressivity then JSX will probably die out anyway. That's why I'm overall pretty positive towards risky and aggressive designs since worst case it'll just go away naturally anyway. The One of the reasons I constantly stay away from using generators in APIs is because of that quirk. That also seems to be the case in JS overall. Maybe it's just that we don't have use cases that require them but I also wonder if people partly stay away from designing APIs to use them because of the ugly star. If I thought it worth while I'd propose dropping it in function generators too. That said, I'd be fine starting off with a Where should it go though? Put it on other statement keywords like <div>*{ let i = 0; for (let item of items) yield <Item index={i} item={item} />; }</div> I prefer Putting it on the inside also seems risky and looks weird outside the JSX context. We already put IMO double curlies is a non-starter: In a way it almost seems less risky to make it implicit since it avoids potential future syntax quirks. JSX can also have breaking changes so it's not necessarily forever if it turns out to be a mistake. Regarding @loganfsmyth's example: const buildElement = co.wrap(function* fn(){
return (
<div class={yield foo()} />
);
}); That's a pretty compelling counter-example if people used generators more. I'm not too worried about this showing up that might in practice because they're so rarely used today. I could probably be convinced that there is a future use case. A sigil would be fine if we could find one that is natural so it sufficiently solves the problem of indicating that something special is going on. Having a very long indicator like double bracing or keywords is probably a non-starter. Also doing nothing at all and leaving this as a PrimaryExpression is not an option IMO. |
I think for generator expressions for JS outside JSX maybe <div>*{ for (let item of items) yield item }</div> <Component
prop="foo"
items=*{ for (let item of items) yield item }
/> Most likely they will be multi-line: <div>
*{
let i = 0;
for (let user of activeUsers) {
i++;
let isEven = (i % 2 === 0);
let id = user.id;
let name = user.name;
if (id && name !== 'DELETED') {
yield <User name={name} id={id} />;
}
}
}
</div> I think the confusing (and breaking) case is when you actually use * as part of JSX text. <div>
This is a paragraph* with some asterisk.
*{explanation}
</div> It would render the explanation without the *. But that's linter space to warn about. |
We should optimize for the "pit of success" - minimizing a super easy-to-stumble-into footgun as "linter space" isn't going to achieve that. |
@ljharb Do you have a concrete concern? Not sure which angle you're referring to. |
@sebmarkbage sorry for the confusion; I was responding to
|
Right. In this case the |
fwiw, that particular lint rule also inspired Forgetting In general, I don't like the implicit nature of this proposal - I think when do expressions exist, |
I honestly don't know how to weigh criticism in syntax discussions. This happens a lot in TC39 too. I feel like in these discussions it either always turn out as 1) fear of new syntax was overly cautious, people learned it quickly and benefited despite concerns. 2) people learned how to use something and benefitted but the overall complexity and gotchas increased. 3) it only increased gotchas and people stopped using it after a while or at least a majority did. Every single addition there is a request for more explicitness given previous history and context. In cases where that happens, every time there is a request to drop the bloated syntax to make it cleaner or a feature isn't used because it is incumbent by bloated syntax. What criteria should we use to determine? It's hard. |
@sebmarkbage tl;dr ahead. Indeed that is an objectively almost impossible task. I'm nitpicking on words perhaps, but it sounds like being explicit avoids your concerns, because it should not increase gotchas as you actually have to choose (single value/function vs push array/generator) and once you choose the behavior is consistent with JS counterparts. I think it also helps readability, to make it clear if what you're looking at is a do-expression or generator function. If you make it implicit, then yes, the user does not need to choose and it just "works". But if they do not know whether they want to emit a single value or a push array then they're just blindly typing code and hoping for the best. I would be genuinely surprised if you wouldn't find at least a handful of people that would exclusively use IMHO, the biggest failure of JS is the Now, it's not the same, but if the TC39 committee had decided to merge functions and generators then it would be a nobrainer to do the same for JSX, but I can only assume they did so for a good reason. If we keep them separate, then it's easy to just point to the documentation of both, many will probably already know it. If you mix the two, you've invented something new that people has to learn that is entirely JSX-specific. As you say, it's a problematic situation. But I think it boils down to a simple question; is JSX an extension of JavaScript, or a designer DSL:ish templating language on-top. If it's an extension of JS then it should be faithful to JS, it's only meant to sand down the rough edges between "React" and JS. If DSL, then go wild and do anything that you think simplifies the lives of "designers". But I don't think it should try to be both. Right now I think it's somewhere in between (with proposals on both sides as well), but still salvageable both ways. Anyway, this is just my perspective on situation and trying to add to the discussion. Do what you want with it. There is no objectively best solution without an unambiguously stated philosophy to benchmark against. So ultimately I think progress is most important and if the discussion is not changing your mind then I have more faith in your vision of what JSX should be over bike-shed "design by committee". |
@sebmarkbage First, let me say that I'm super happy there is actively being worked and thought about this. My main concern is that JSX works differently then Ecmascript. I think it is worth discussing if we can "improve" the syntax for generators. If C# implicitly uses But I would prefer that JSX and Ecmascript works the same. So, if JSX doesn't require a star, but uses the For example, I would expect the below two are valid syntax: let gen = (() => {
yield 1;
yield 2;
})();
let gen = do {
yield 1;
yield 2;
}; If so, this proposal is not really needed anymore: You could then also in ecmascript just write: let arrayOfUsers = [...do {
for (let user of users)
if (user.name.startsWith('A'))
yield user;
}]; This would do then the exact same as the proposed |
@ljharb This is not true, you can not create generators with the current do expression proposal, and babel plugin. For the non-generator case, there is one big annoyance as it comes to do expressions in JSX templates. It gets very noisy quickly and you get a lot of indentation. For example, consider this example: <div className="col">
{
do {
if (timer == null) {
<button className="btn btn-info btn-lg" onClick={this.startTimer}>
Start Timer
</button>;
} else {
<button className="btn btn-danger btn-lg" onClick={this.endTimer}>
End Timer
</button>;
}
}
}
</div> With this proposal, you have less noise and 4 spaces less indentation: <div className="col">
{if (today.breaks.current() == null) {
<button className="btn btn-info btn-lg" onClick={this.startTimer}>
Start Timer
</button>;
} else {
<button className="btn btn-danger btn-lg" onClick={this.endTimer}>
End Timer
</button>;
}}
</div> This is also depends on how prettier would handle this, and maybe prettier could parse do expressions, so that it looks less noisy. But I do think this proposal would be a win readability. |
@kasperpeulen i'm talking about the part where the last value is returned. If jsx wanted to add |
@sebmarkbage How does this look:
Seems like we might need the additional I'm going to put up a new proposal based on the feedback in this thread, but please feel free to keep discussing this one here. |
Wondering what people's thoughts are on keeping the old JSXExpression semantics when using |
@clemmy i think adding support in jsx for something that’s not even stage 1 yet (ie, it might not make it into the language at all) is premature; but i like that much much better than implicit alternatives. |
Closing this issue for now in lieu oF explicit generator expressions. |
RFC: React Expressions with Implicit Do-Generator Semantics
Background
A highly requested feature of React seems to be a nicer way to loop and write conditionals inside JSX. For example this StackOverflow question about loops in JSX has over 300k views, and there are popular libraries such as jsx-control-statements written in order to address this fundamental feature of a templating language. This fostered discussions such as #39, #88, and reactjs/react-future#35, suggesting syntax in order to allow for similar capabilities in JSX. With the new JSX semantics, you'd be able to do something like this:
while keeping the old behavior as well.
Proposal
The proposal is to use a combination of implicit do expression semantics as well as implicit generator expression semantics to fulfill a wide range of use cases. (By implicit, I mean omitting the
do
in do-expressions and the*
in generator expressions). We'll extend the grammar for theJSXExpression
block, to accept a list of statements. These statements are what you would usually find inside aBlock
, but it also allows theyield
keyword. Additionally, there will be the following criteria:If it starts with
{
, then that is anObjectLiteral
, not aBlockStatement
(Ignoring leading comments and open parenthesis). Similarly, if it starts with a (async) function expression, class expression, or generator expression, then it is of that following type.continue
is not allowed at the top scope. They are allowed inside a nested block as long as they don't reference at a higher scope level than their own.return
is not allowed except inside nested functions.This allows for code like the following:
The semantics for the evaluation of the code in-between the braces
{...}
are as follows:Test cases
https://gist.github.com/clemmy/f0de896e4853172d88f664ab23c517d9
Alternative Considerations
@sebmarkbage and I converged on the proposal I outlined above after a lot of discussion. At first glance, it probably looks a bit weird since the behavior of the same syntax differs depending on whether a yield is present or not, but hey, that's better than one of our thoughts to make that behavior dynamic based on runtime. 🙂 Here are some of the alternative proposals we considered:
Always using generator expression semantics:
Cons:
This would be a breaking change with current JSX since yields will always be returned as an array; the following wouldn't be valid anymore:
In order to fix this, we use a modified generator expression semantics approach:
Always using generator expressions semantics # 2
Cons:
The behavior is dictated by the runtime (e.g. whether we return
completion
oryields
is based on the runtime evaluation of the length ofyields
).Do Expression by default and explicit generator expression syntax
For example, we can use
[ ... ]
to indicate that it's a generator expression. This allows the following code, which is actually quite nice:Cons:
This would break existing JSX where an opening
[
is a part of the text insideJSXChildren
.Do Expression by default and explicit generator expression syntax # 2
Similar to the above
[ ... ]
, but we use a star instead (*{ ... }
). It was hard to decide between this one and the main proposal.Related Links