JSX Transformer / DisplayName Visitor: Multiple declarations with one var statement

[seiffert]

The bug fixed by this commit prevented the correct parsing of var statements with multiple variables being declared. Instead of trying to parse a whole 'variable declarations' (a var statement with all its declarations), this visitor now only parses single 'variable declarators'.

An example (from the tutorial) that did not work before this patch:

    var CommentBox = React.createClass({
      ...
    }),

    CommentList = React.createClass({
      ...
    });

What did work is the following:

    var CommentBox = React.createClass({
      ...
    });

    var CommentList = React.createClass({
      ...
    });

Of course, there are different coding styles and guidelines. Most developers probably don't like multiple variables being declared with one var statement, but nevertheless, it should be possible.

[jordwalke]

:like: I hadn't considered this case. I'm curious how you are using the displayName. We have a bookmarklet that will annotate the page with visual labels on top of the DOM that tell you the names of the components that generated that part of the page. I'll see if we can get that checked in.

[seiffert]

Oh actually I'm not using displayName myself, the page just won't render because javascript dies with an Uncaught SyntaxError: Unexpected token <. The output of fbtransform still contains tag literals:

      var CommentBox = React.createClass({
        ...
      }),

      CommentList = React.createClass({
        render: function() {
          var comments = this.props.data.map(function (comment) {
            return <Comment author={comment.author}>{comment.text}</Comment>;
          });

          return (
            <div class="commentList">
              {comments}
            </div>
          );
        }
      });

(the body of CommentBox is omitted since it is transformed correctly.)

[seiffert]

And: I would really be interested in this bookmarklet! :)

[jeffmo]

Hmm, I'm having a hard time reproing this issue... I made repro.js:

/**
 * @jsx React.DOM
 */

var CommentBox = React.createClass({render: function() {}}),

        CommentList = React.createClass({render: function() {}});

and when I run ./bin/jsx repro.js I get this:

/**
 * @jsx React.DOM
 */

var CommentBox = React.createClass({displayName: 'CommentBox',render: function() {}}),

        CommentList = React.createClass({displayName: 'CommentList',render: function() {}});

Did you include the "@jsx" tag in the docblock at the top? Or maybe I'm missing something?

[seiffert]

@jeffmo Could you please try to make render return a DOM component? The transformation of this JSX code to javascript is what doesn't work for me... The transformation should happen in browser-transforms.js on line 39. However, after executing this line, the variable functionBody holds the string contained in this gist: https://gist.github.com/seiffert/5681370.
What is happening is that the reactDisplayName visitor iterates over all declarations included in the var. As far as I understand, it should only add the component's displayName property to its initializer's first argument. What it actually does is the following: For every declaration, it copies the script from the current pointer to the first argument of the declaration's initializer into the script buffer. However, this step copies all contents from the last declaration's initializer's first argument's position (stored in state.g.position) to the current declaration's initializer's first argument into the buffer. This effectively copies the whole untransformed initializer into the script buffer.
After this, the normal transformation takes place which leads to the correct but somehow ~duplicate output at the bottom...

Yes, I do have a @jsx React.DOM tag in the docblock at the top of my <script> tag - see the whole HTML document here.

[seiffert]

I tried to reproduce the problem with the jsx binary - it does not occur! WIth the version of JSXTransformer.js installed via bower install react, the problem can be reproduced...

[jeffmo]

Aha you're right! It's because we move past the contents of the object literal in the displayName transform step before the react.js transform has a chance to run on the declaration node.

Ok, do you mind filling out our CLA and then I can merge this in for you:
https://developers.facebook.com/opensource/cla

[jeffmo]

This first conditional is no longer necessary since it's checked in visitReactDisplayName.test() -- mind killing it?

[seiffert]

True, if we can rely on test() being called before visitReactDisplayName(), we can remove that first condition!

[jeffmo]

This looks good to after we clear up line 39 -- thanks a ton for the fix!

[seiffert]

I already signed the CLA!