Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Appearance settings

Conversation

GeoffreyBooth
Copy link
Collaborator

render = ->
  <Row>a</Row>
  <Row>b</Row>
[stdin]:3:4: error: Adjacent JSX elements must be wrapped in an enclosing tag
  <Row>b</Row>
   ^^^^^^^^^^^

Not sure why the initial < isn’t part of the error highlight, but that’s a separate bug. @xixixao

@GeoffreyBooth GeoffreyBooth requested a review from zdenko April 30, 2018 03:33
@GeoffreyBooth GeoffreyBooth merged commit fe75548 into jashkenas:master May 1, 2018
@GeoffreyBooth GeoffreyBooth deleted the fix-jsx-return branch May 1, 2018 15:09
@vendethiel
Copy link
Collaborator

It seems to check a bit too many cases, though:

$ echo -e "->\n  <a></a>\n  foo\n  <b></b>" | ./bin/coffee -bcs
[stdin]:4:4: error: Adjacent JSX elements must be wrapped in an enclosing tag
  <b></b>
   ^^^^^^

This is probably OK (why would you put a random toplevel element), but means we check every child of a block to check if it's a JSX element, if the returned value is a JSX element, which might be a bit wasteful.

@GeoffreyBooth
Copy link
Collaborator Author

It only checks siblings if the value to be returned is a JSX element. So yes, it’s a little wasteful for whenever you’re returning a JSX element, but I don’t know how to avoid checking every element if siblings are disallowed.

But I think the bigger problem is that this:

var render = () => {
  <a></a>
  <b></b>
}

throws the Adjacent JSX elements must be wrapped in an enclosing tag error in Babel, whereas this:

var render = () => {
  <a></a>;
  <b></b>;
}

does not. I guess it’s because the first example’s lack of semicolons makes it equivalent to this:

var render = () => {
  <a></a><b></b>
}

which throws the error.

So what to do? It would appear that this PR has made our error checking too strict, since this:

var render = () => {
  <a></a>;
  return <b></b>;
}

should be allowable JSX, at least if Babel is to be trusted.

@vendethiel
Copy link
Collaborator

Well, we can loop from 1 to length, and if both nodes[i]...csx && nodes[i - 1]...csx, we have found adjacent tags.

zdenko added a commit to zdenko/coffeescript that referenced this pull request May 2, 2018
@zdenko zdenko mentioned this pull request May 2, 2018
GeoffreyBooth pushed a commit that referenced this pull request May 20, 2018
* Fix #5046: Adjacent JSX

* check CSX only when wrapped in parentheses

* Fix indentation

* Add test for unlikely, but valid, JSX syntax
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.

3 participants

Morty Proxy This is a proxified and sanitized view of the page, visit original site.