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

JSXStatement - What does it mean? #87

Open
sebmarkbage opened this issue Aug 18, 2017 · 16 comments
Open

JSXStatement - What does it mean? #87

sebmarkbage opened this issue Aug 18, 2017 · 16 comments

Comments

@sebmarkbage
Copy link
Contributor

sebmarkbage commented Aug 18, 2017

This is building on top of discussions in #85 and #86. However those include a lot of meta discussion, I'd like to clarify something much more narrow before we go into how it fits and whether it's a good idea.

I think there is a reasonable motivation for doing implicit return contrary to other JS contexts requiring explicit return. An arrow function without curly braces with a do expression isn't much different. I could argue that if anything JS should've had this for all functions and this is just a special case of fixing it. I see the points of making the language difficult to understand overall but there's already confusion why you can't just do this. JSX is seen as something slightly different. Not necessarily as just expressions. We don't have to decide on this now.

The semantics of JSX expression nor a statement is not up to this specification. It can be implemented differently in different transpilers and such. This repo is just meant to define the syntax without semantics.

JSX is already allowed as an expression statement. I'm not so concerned about breaking changes in this position at this point.

What I'd would like to clarify exactly what syntax differences would making a JSXStatement with no other changes include at this point?

@syranide you mentioned that it would affect ASI. Can you elaborate on that?

cc @bmeck

@syranide
Copy link
Contributor

@sebmarkbage Can you clarify what you mean by ASI?

@sebmarkbage
Copy link
Contributor Author

sebmarkbage commented Aug 18, 2017

Automatic Semicolon Insertion

@syranide
Copy link
Contributor

syranide commented Aug 18, 2017

@sebmarkbage Ah ofc. What I posted in that issue:

foo()
<bar /> // implying implicit return

Would get tokenized to "foo" "()" "<" "bar" "/>" and thus normally parsed as foo() < bar .... AFAIK the only way around that is to special-case is somehow, either through ASI-rules or by making exceptions in the parser. So it can be worked around, but not "solved". Unless I'm mistaken?

@bmeck
Copy link

bmeck commented Aug 18, 2017

Can you clarify why foo() and <bar /> are related. Currently ASI would keep them as separate ExpressionStatements:

http://astexplorer.net/#/gist/8fcdde72ae0863f9a0947fb81774338e/cd254fb0b8c50f7711dc75f7f10b5398303b6ea9

@syranide
Copy link
Contributor

@bmeck Remove the semicolon, this is about implicit semicolon:
http://astexplorer.net/#/gist/8fcdde72ae0863f9a0947fb81774338e/56027b28ba324ac2a492fd62e3302aeff8834a3f

@bmeck
Copy link

bmeck commented Aug 18, 2017

Ah, so it fails to parse when it would be in a position that is potentially given an ASI. I would state not omitting semicolons would be a good thing since it doesn't have this problem.

Since this is an early error and doesn't allow code to evaluate/parse. I would throw this on the pile of ASI oddities unless there are good reasons to fix it?

Edit: the same class of oddities that need you to add semicolons before leading operators, [, and (.

@sebmarkbage
Copy link
Contributor Author

We would need a spec mechanism to fix it while being backwards compatible. Since things like this is already valid JS:

foo()
<i>text</i></i>
bar()

@bmeck
Copy link

bmeck commented Aug 18, 2017

@sebmarkbage that is a good example for backwards compatibility. Nested and adjacent JSXElements always have >< I think though. Is there an example of nested/adjacent grammar collision?

foo()
<i>
<i>text</i></i>
bar()

Fails.

Also, is there an example that is in a completion position given to a function body where implicit return is being looked at.

@syranide
Copy link
Contributor

syranide commented Aug 18, 2017

I think there is a reasonable motivation for doing implicit return contrary to other JS contexts requiring explicit return. An arrow function without curly braces with a do expression isn't much different. I could argue that if anything JS should've had this for all functions and this is just a special case of fixing it. I see the points of making the language difficult to understand overall but there's already confusion why you can't just do this. JSX is seen as something slightly different. Not necessarily as just expressions. We don't have to decide on this now.

@sebmarkbage I'm not sure if I fully understand what part of the previous discussions you're referring to here. That return <A /> and <A /> are the same? Or that <A>{if (cond) <B/>}</A> should implicitly return <B />? If it's the first then I don't think I understand your reasoning. If it's the second then I fully agree unless the it turns out to be very problematic, as per earlier discussions I think JSX could greatly benefit from (something like) it.

@sebmarkbage
Copy link
Contributor Author

@syranide I don't even want to have that discussion here yet. I'm not proposing any semantics yet. I'm just saying that's plausible and therefore we could consider carving out a JSXStatement in the syntax without semantics to allow that experiment and discussion to happen elsewhere. That could mean throwing, returning, goto, or whatever a transpiler thinks it should do.

@syranide
Copy link
Contributor

@sebmarkbage Ah ok, now I'm with you. 👍

@sebmarkbage
Copy link
Contributor Author

@bmeck What I'm referring to is JS without JSX has that as valid syntax. Even if it's technically not ambiguous it would require a lot of look ahead to see if it is and cover grammar doesn't work for these.

@syranide
Copy link
Contributor

syranide commented Aug 18, 2017

What I'm referring to is JS without JSX has that as valid syntax. Even if it's technically not ambiguous it would require a lot of look ahead to see if it is and cover grammar doesn't work for these.

@sebmarkbage I would assume the solution here would simply be to special-case < after an ASI-candidate newline; if there's < and a valid JSX-identifier (without space) then it is a JSXStatement (preceded by an ASI). I don't see any other way to disambiguate these two that would actually make sense. Looking further ahead and trying to see if it's valid JSX-element syntax is bad for all kinds of reasons (even if we could do it). You could possibly stretch it to check for the next token after the JSXIdentifier too, but I don't think it would really matter.

It's not horrible, but it's not very nice either.

EDIT:

foo()
<bar /> // JSXStatement
foo()
< bar /> // Comparison and unexpected token
foo();<bar /> // JSXStatement
foo()<bar /> // Comparison and unexpected token

EDIT: So minifiers would be required to insert an explicit semicolon for it to be safely recognized as a JSXStatement.

@sebmarkbage
Copy link
Contributor Author

Minifiers/printers sometimes insert line breaks at pretty arbitrary places just to avoid too giant lines. That code wouldn't have any spaces in it. I could see this being a breaking change that would mean that we couldn't parse some existing code using the same parser.

I think this would simply not be treated as JSX without the preceding semi-colon. Which is the same as today. So no change needed.

@not-an-aardvark
Copy link

I found JSX easy to learn after knowing JS because "A JSXElement is just an expression" is a very simple mental model to understand. To me, it seems like introducing additional semantics such as implicit return would lead to increased confusion over what the syntax is doing, with no clear justification other than saving a few keystrokes from typing return. I think it's unlikely that something like foo.map(X => { <X />; }) is a big point of confusion for people when writing JSX in particular, because people who are confused by that would also end up confused by something like foo.map(value => { value * 2; }) anyway.

@sebmarkbage
Copy link
Contributor Author

It seems to me that this is an issue for JSX in do-expressions too.

It also seems like it's non-trivial to try to fix this. It might be that you simply have to add a semi-colon for this use case.

If there is nothing we can do to fix this then do we really need JSXStatement as a special syntactical form. Isn't JSXElement in an ExpressionStatement enough?

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

No branches or pull requests

4 participants