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

Trim whitespace from end of tags to fix linter warnings. #682

Closed
wants to merge 1 commit into from
Closed

Trim whitespace from end of tags to fix linter warnings. #682

wants to merge 1 commit into from

Conversation

STRML
Copy link
Contributor

@STRML STRML commented Dec 20, 2013

I'm building SublimeLinter-jsxhint for proper JSX linting in Sublime Text 3. Unfortunately we get trailing whitespace issues at the end of tags with the current version of react.

Deleting the trailing whitespace at L135 fixes the linter warnings and does not affect the output.

@zpao
Copy link
Member

zpao commented Dec 20, 2013

cc @jeffmo who can help with transform-related code.

Personally, I think we might want to be a bit less naive and forward look to see if we're at the end of a line.

Take this for example:

<div id="foo">
  Hello
  <span>{this.props.name}</span>
</div>

It gets transformed to this before your change:

React.DOM.div( {id:"foo"}, 
  " Hello ",
  React.DOM.span(null, this.props.name)
)

There is a trailing space on line 2 but there’s also a space after null,

With your change we blindly lose that space for inline children, which will probably make somebody else’s linter complain. If we’re going to change this at all, I think we should do it completely.

@ghost ghost assigned jeffmo Dec 28, 2013
@syranide
Copy link
Contributor

I could possibly look into this, but I would recommend waiting for #480 to ship as it already touches most of JSXTransformer.

@zpao
Copy link
Member

zpao commented Feb 14, 2014

@STRML, are you still interested in doing this?

@syranide
Copy link
Contributor

@zpao #970

@chenglou
Copy link
Contributor

(Nvm, ignore this, not fixed).

@parshap
Copy link

parshap commented Mar 7, 2014

👍

@zpao
Copy link
Member

zpao commented Mar 12, 2014

Closing this out in favor of #970. @syranide correct me if that wasn't right!

@zpao zpao closed this Mar 12, 2014
@syranide
Copy link
Contributor

@zpao Correct :)

@facebook-github-bot
Copy link

@STRML updated the pull request.

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.

None yet

7 participants