Tag name for jsxtransformer cannot be "namespaced"

[jtmalinowski]

$ jsx --version
0.8.5

/** @jsx React.DOM */
var HelloMessage = React.createClass({
  render: function() {
    return <Component name={this.props.name} />;
  }
});

But when Component is a field of an other object:

/** @jsx React.DOM */
var HelloMessage = React.createClass({
  render: function() {
    return <Nmspc.Component name={this.props.name} />;
  }
});

it fails with

Error: Parse Error: Line 4: XJS tag name can not be empty
at throwError (/home/developer/.nvm/v0.10.13/lib/node_modules/react-tools/node_modules/esprima/esprima.js:2137:21)
at parseXJSIdentifier (/home/developer/.nvm/v0.10.13/lib/node_modules/react-tools/node_modules/esprima/esprima.js:5245:13)
at /home/developer/.nvm/v0.10.13/lib/node_modules/react-tools/node_modules/esprima/esprima.js:5717:38
at parseXJSAttribute (/home/developer/.nvm/v0.10.13/lib/node_modules/react-tools/node_modules/esprima/esprima.js:5305:16)
at /home/developer/.nvm/v0.10.13/lib/node_modules/react-tools/node_modules/esprima/esprima.js:5717:38
at parseXJSOpeningElement (/home/developer/.nvm/v0.10.13/lib/node_modules/react-tools/node_modules/esprima/esprima.js:5356:29)
at /home/developer/.nvm/v0.10.13/lib/node_modules/react-tools/node_modules/esprima/esprima.js:5717:38
at parseXJSElement (/home/developer/.nvm/v0.10.13/lib/node_modules/react-tools/node_modules/esprima/esprima.js:5375:26)
at /home/developer/.nvm/v0.10.13/lib/node_modules/react-tools/node_modules/esprima/esprima.js:5717:38
at parsePrimaryExpression (/home/developer/.nvm/v0.10.13/lib/node_modules/react-tools/node_modules/esprima/esprima.js:2651:20)

(nvm here is a node version manager, just like rvm but for node, you can find it on github)

I hope it's not as RTFM case :)
Even if it's not supported currently, it would be a nice feature IMHO. I'm not that familiar with the code to come up with a PR unfortunately.

[vjeux]

We talked about it several times and we agreed that we should support <Namespace.Component> but no one actually wrote the code to support it yet.

In the meantime you can do
var Component = Namespace.Component;

Thanks for the report :)

[sophiebits]

(See #74.)

[jtmalinowski]

I start holidays next week, so I'll try to take a stab at it if it still won't be resolved.

[jtmalinowski]

@vjeux @zpao what do you think about https://github.com/jakubmal/esprima/tree/namespaced-jsx-tags I based it on commit a3e0ea3979eb8d54d8bfade220c272903f928b1e in facebook/esprima, because there is a lot of new commits in a3e0eea...master, so we either need to make a branch in facebook/esprima, or update version of esprima used in react.

@vjeux as you said on irc, the fix is really a one-liner, maybe guys from facebook/esprima will want to have more tests on this, but apart from that, really easy.

[zpao]

@jeffmo, this is all you on the esprima side. We'll obviously want to make sure this change is fine internally before we take it.

@JakubMal Feel free to open PRs against facebook/esprima#fb-harmony (that's the branch we use internally and here in React - npm has git dependency issues, that's why we point at a tarball for a rev in our package.json). I think we intentionally lag behind master since we've done some considerable work in our branch, though @jeffmo knows better. We'll almost certainly want some tests :) We didn't add our other tests in vendor/fbtransform/, but we have some other React specific ones and we'd want to add some internally before we point React at a new rev.

[jeffmo]

I think this is mostly fine, but I would like for us to continue using xml semantics if we want to go down this road. So instead of <Namespace.Component /> I'd rather see us use : a la <Namespace:Component />

[vjeux]

@JakubMal: I said that it shouldn't be too hard. But I'm pretty sure it's not a one liner :)

[jeffmo]

Going with : (as opposed to .) would allow us to separate the meaning of the syntax from the output of the compilation pipeline and leave the "what does this mean?" decision to the compiler. This is nice because it would be pretty reasonable to parameterize the compiler (with some reasonable default) in terms of deciding what something like <MyStuff:MyThing /> should de-sugar to.

i.e. Does it compile to MyStuff.MyThing? Or maybe MyStuff__MyThing? Or hell maybe even require("MyStuff/MyThing")

If we go with ., we're making a fairly hard-to-reverse decision that we want JSX namespacing to always be tied to a MemberExpression (because what else would <MyStuff.MyThing /> mean?)

[chenglou]

Is it too late to voice an opinion?

I'm not so sure if implementing namespace is a good idea, for three reasons:

1. How necessary is it? Maybe this is short-sighted, but I don't see any benefit of NS.DatePicker over NSDatePicker. In fact, the latter is shorter and requires absolutely nothing extra to learn.
2. I get that it's an entirely optional feature, but this will get documented and presents more of a mental hurdle to developers than anything else. JSX's already getting weird eyes every time it gets mentioned, even though it's repeated throughout the documentation that it's optional.
3. JSX is just JavaScript, i.e. there'll be lots of opportunities to shape DOM as what fits JS, which might not be a good idea and reminds me of the typical templating engine that's trying too hard to emulate stuff. Plus, say I write NS:DatePicker and it translates into NS__DatePicker, how would that help in making JSX optional? I'll have to learn the backward way and remember that in vanilla JS NS__DatePicker conforms to what JSX compiles down to; there's not much logic in that and while we're at it, why not just write NSDatePicker.

[jtmalinowski]

Well,

1. Not necessary
2. No need to document that, at least not in any tutorial, it might be mentioned somewhere deeper.
3. I did not suggest such a magic, it doesn't make sense if JSX is not coupled with any dependency tool

I suggested something extremely simple like:

<App.Views.Counter /> 

to translate to App.Views.Counter({})

And I suggested to simply include dot (.) as allowed char in tag name, then the whole tag name including dots is copy&pasted into resulting JS. I believe it doesn't have any wider implications if you commit to reserving dot for this case, and you're still left with a colon.

It might help in say 2-3% of all the situations, so if it is such a hassle, then let's just leave it.

[jeffmo]

With regard to including dot as an allowed character in the tag name: This not the right approach for the parser. If the intent of <App.Views.Counter /> is to be a javascript member expression, then it should be represented as such in the AST (so that no further parsing must be done when the AST is interpreted). App.Views.Counter is not a valid identifier -- nor is that even its intended meaning (the intended meaning is a member expression).

On the . vs : front: . locks us in to "namespacing" meaning "nested objects". This restricts our option value for any future scenarios where we might want to implement namespace resolution in other ways (such as nested modules). So barring any other significantly compelling arguments I'm going to be the bad guy and say that : is the correct punctuator to be used here if we want to move forward with this feature.

[jeffmo]

(I should note that I also don't feel terribly strongly about the need for this feature -- but if done in a future-friendly way, I'm willing to take the pull request)

[jtmalinowski]

I am not really sure, could you clarify? How is it that dot is locking us into "nested objects"? If "nested objects" as tag names are implemented using colon then it locks colon into this meaning in the same way it would lock dot into this meaning. It is analogical, isn't it. Please, let me understand it here.

Given #74 and implementing nested objects as member expression do you still think it is not the right solution? If not, then closing.

[jeffmo]

To be clear I wasn't suggesting that we parse colon delimiters into member expressions, but just that we parse them into some kind of "namespace" structure in the syntax tree so they can be interpreted more freely by the compiler.

If we used <App.View /> and parsed that into a member expression, you've got a member expression to deal with pretty much forever (changing what this means will be rough on backward compatibility). If we parsed it generically and compiled it in to something other than a member expression, it might be non-intuitive for unfamiliar readers of the code because, as a reader, my thought process would go something like: Well it's clearly not valid xml, and it looks like a js member expression...so I'll assume this compiles to a member expression lookup for the component.

Whereas <App:View /> is less semantically tied to a particular output result in terms of what "namespacing" means, its still valid xml (good for readers of the code who are familiar with xml), and it can be defined however you see fit in your particular application (easily parameterizable in the jsx compiler) with a default of something reasonable like a member expression.

[jtmalinowski]

In that case (parametrizable namespaces), right now I don't have the amount of time it would take to implement it (conditional member expression parsing, tests, cli switch, react-rails config&tests off the top of my head)

[lrowe]

Using a colon as a separator conventionally locks you into a single level of namespacing - <App:Views:Counter /> looks odd.

Perhaps allowing the tag name to be supplied as an expression would provide the flexibility desired? <{App.Views.Counter} />

This would allow even complex tag expressions to be expressed clearly, e.g. <{App.registered_views[obj.item_type]} /> or <{obj.item_type == 'special' ? SpecialView : DefaultView} />.

[jeffmo]

@lrowe: Its possible to represent an arbitrarily deep number of nested namespaces as such:

<App.Views.Counter />

{
  type: 'XJSElement',
  openingElement: {
    type: 'XJSOpeningElement',
    name: {
      type: 'XJSIdentifier',
      namespace: {
        type: 'XJSIdentifier',
        namespace: {
          type: 'XJSIdentifier',
          namespace: null,
          name: 'App'
        },
        name: 'Views'
      },
      name: 'Counter'
    },
    [...]
  },
  [...]
}

This turns out to be true regardless of what the namespacing delimiter is -- my only issue with . is just that it would deviate from established xml convention and it would look too closely like a member expression which may not always be what we want namespaced components to compile to.

[vjeux]

@lrowe You can already do what you want using a temporary variable as follows:

var View = App.registered_views[obj.item_type];
<View />

var View = obj.item_type == 'special' ? SpecialView : DefaultView;
<View />

This also enables you to add children to it:

var View = obj.item_type == 'special' ? SpecialView : DefaultView;
<View>
  <SomeChildren>
</View>

Otherwise there's an issue where it's unclear what you put in the end tag:

<{obj.item_type == 'special' ? SpecialView : DefaultView}>
  <Children>
</{obj.item_type == 'special' ? SpecialView : DefaultView}>