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 compiler leaves trailing whitespace in object definitions #3553

Closed
hatched opened this issue Mar 31, 2015 · 9 comments
Closed

jsx compiler leaves trailing whitespace in object definitions #3553

hatched opened this issue Mar 31, 2015 · 9 comments

Comments

@hatched
Copy link

hatched commented Mar 31, 2015

Assuming the following render call.

      React.render(
          <views.AddedServicesButton
            serviceCount={serviceCount}
            closed={closed}
            changeState={this.__changeState.bind(this)}/>, container);

The generated output.

      React.render(
          React.createElement(views.AddedServicesButton, {
            serviceCount: serviceCount, 
            closed: closed, 
            changeState: this.__changeState.bind(this)}), container);

You'll notice that there is a trailing white space on the serviceCount and closed properties which is causing issues for linters which don't allow trailing white space.

@jimfb
Copy link
Contributor

jimfb commented Mar 31, 2015

You should be running your linters on source code (not generated code). Generated code generally should not be checked into a repository, should not be linted, etc. For this reason, this is probably pretty low priority. That said, if you'd like to submit a pull request, it would likely be considered.

@sophiebits
Copy link
Collaborator

@JSFB We've tended to recommend linting after running through JSX as that's what FB does and it preserves line numbers, etc.

@jimfb
Copy link
Contributor

jimfb commented Mar 31, 2015

Eeeh, That would mean that if someone's lint rules state that there must be a space between a function name and the open pren, that the linter would fail. Or any number of potential lint rules that happen to conflict with whatever the transpiler produces. You can't win, because people's linting rules can be mutually exclusive and thus there is no possible transpiler output that will pass all linters. Better to lint what the human is actually reading/editing.

@sophiebits
Copy link
Collaborator

It's true, but then your linter needs to understand JSX. Now there are some (particularly eslint) but there weren't a year ago.

@jimfb
Copy link
Contributor

jimfb commented Mar 31, 2015

Yep, I agree :).

@hatched
Copy link
Author

hatched commented Apr 1, 2015

I agree that supporting the all of the various linting options would be an exercise in futility. I brought this one up because it appears to be a bug being that it doesn't add the trailing white space in any other instance I could find.

@jimfb
Copy link
Contributor

jimfb commented Apr 1, 2015

Yeah, having the transpiler produce trailing whitespace probably wasn't intentional, but it's not a high-priority bug (primarily since it's generated code and thus not intended for human consumption). Like I said, we'd likely accept a pull request if you'd like to fix it :).

@syranide
Copy link
Contributor

syranide commented Apr 1, 2015

#970, it's non-trivial to fix in the generator and was left as-is for now.

@jimfb
Copy link
Contributor

jimfb commented Oct 15, 2015

JSX compilation is now handled by babel, which I think renders this issue no longer applicable.

@jimfb jimfb closed this as completed Oct 15, 2015
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

5 participants