Skip to content

A snippet that doesn't compile down to pretty jsx #239

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

Closed
chenglou opened this issue Jul 29, 2013 · 3 comments
Closed

A snippet that doesn't compile down to pretty jsx #239

chenglou opened this issue Jul 29, 2013 · 3 comments

Comments

@chenglou
Copy link
Contributor

Currently jsx transforms this (with the indentation):

            if (this.props.completedCount > 0) {
                clearButton = (
                    <button
                        id="clear-completed"
                        onClick={this.props.onClearCompleted}>
                        Clear completed ({this.props.completedCount})
                    </button>
                );
            }

into this:

            if (this.props.completedCount > 0) {
                clearButton = (
                    React.DOM.button(
                        {id:"clear-completed",
                        onClick:this.props.onClearCompleted}, [
" Clear completed (",this.props.completedCount,") "                 ])
                );
            }

Which fails to pass jshint because of the indentation (the temporary solution is to put {''} before and after the bad line). It gets worse if you prefer the style of putting the > on a new line, it becomes:

            if (this.props.completedCount > 0) {
                clearButton = (
                    React.DOM.button(
                        {id:"clear-completed",
                        onClick:this.props.onClearCompleted}
                    , [
" Clear completed (",this.props.completedCount,") "                 ])
                );
            }

Which fails two additional tests, the comma style (that can be disabled with laxcomma and bad line breaking (not sure if it can be disabled).

@zpao
Copy link
Member

zpao commented Aug 14, 2013

Yea, this is pretty awkward. Since we do pretty dumb string transforms and preserve line numbers, it's not going to be pretty. My long term goal is to get to AST->AST transforms with sourcemaps and pretty code generation with something like escodegen.

We could at least do better with comma style in the short term.

@sophiebits
Copy link
Collaborator

I made some changes a while back so now you get:

            if (this.props.completedCount > 0) {
                clearButton = (
                    React.DOM.button(
                        {id:"clear-completed",
                        onClick:this.props.onClearCompleted}, 
                        " Clear completed (",this.props.completedCount,") "
                    )
                );
            }

@zpao
Copy link
Member

zpao commented Nov 9, 2013

If somebody wants to tackle the comma part of this, then go for it but otherwise, let's close it out.

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