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

Clarify JSX/React WhiteSpace rules #40

Closed
basarat opened this issue Aug 20, 2015 · 6 comments
Closed

Clarify JSX/React WhiteSpace rules #40

basarat opened this issue Aug 20, 2015 · 6 comments

Comments

@basarat
Copy link

basarat commented Aug 20, 2015

Based on the following test cases:

var p = 0;
// Emit "   "
<div>   </div>;
// Emit "  ", p, "   "
<div>  {p}    </div>;
// Emit only p
<div>  
      {p}    
      </div>;

// Emit only p
<div>
  {p}
    </div>;

// Emit "  3"
<div>  3  
  </div>;

// Emit "  3  "
<div>  3  </div>;

// Emit "3"
<div>   
   3    
   </div>;

// Emit no args
<div>   
   </div>;

// Emit "foo" + ' ' + "bar"
<div>  

   foo

 bar   

  </div>;

// Emit "foo  " + "bar" + "  baz"
<div>foo  <span>bar</span>  baz</div>

// Emit "foo  bar"
<div>foo        bar</div>;

For JSXText (https://facebook.github.io/jsx/) I've drawn the following set of rules:

  1. Trims whitespace in the following cases (mentioned node might be a parent or a sibling):
    • leading whitespace if this is the first node on the same line
    • trailing whitespace if this is the last node on the same line
  2. Inserts a single whitespace if the next sibling is also JSXText and not on the same line
  3. Any whitespace tabs (alt09) is replaced with a single space demo

Am I missing something ... and is this already documented elsewhere (specifically 2 wasn't mentioned explicitly anywhere I could find). Need this information to help with TypeScript/TSX integration 🌹

I know this is specific to React Emit for JSX but this seemed like the more appropriate repo 😃

PS : I've seen http://facebook.github.io/react/blog/2014/02/20/react-v0.9.html#jsx-whitespace

@syranide
Copy link
Contributor

facebook/react#480 (comment), there is also the full technical reasoning facebook/react#480, but it may be a bit hard to digest.

The idea being that indentation is not intended for output and trailing whitespace is a bad idea all-around. 2 is added because that is almost universally what you want and not having it is really really cumbersome and mistake-prone.

@basarat
Copy link
Author

basarat commented Aug 21, 2015

@syranide reading that I can only see that there is one more rule regarding tab handling. I've updated the original question with point 3, and also see demo

Cheers 🌹

@basarat basarat closed this as completed Aug 21, 2015
@RReverser
Copy link
Contributor

This question is being raised rather often, I'm thinking maybe we should add some kind of react.md in this repo?

@basarat
Copy link
Author

basarat commented Aug 24, 2015

I'm thinking maybe we should add some kind of react.md in this repo?

I would prefer whitespace.md. Even though the thing I am discussing feels react emit specific, but I feel that not having whitespace rules specified in jsx decreases its portability.

Consider:

<div> 
test
</div>

Would be weird if something things see it as test (like react) and other's see it as \ntest\n. But react.md is fine as well :)

@syranide
Copy link
Contributor

@basarat "other's see it as \ntest\n", which ones are that?

@basarat
Copy link
Author

basarat commented Aug 24, 2015

"other's see it as \ntest\n", which ones are that?

Other implementations that might use JSX for an HTML emit

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

3 participants