Stop requiring docblock for JSX transformer #114

Closed
zpao opened this Issue Jun 20, 2013 · 9 comments

Comments

Projects
None yet
6 participants
@zpao
Member

zpao commented Jun 20, 2013

/** @jsx React.DOM */ is pretty annoying noise. Let's stop requiring it. We can probably keep it simple for now but if we want to make JSX more generic and standalone we might need to do this a bit more carefully.

@jordwalke

This comment has been minimized.

Show comment
Hide comment
@jordwalke

jordwalke Jun 20, 2013

Member

👍

What are you thinking as a substitute? We should cover in-browser transforms as well as something like require.js (with or without in-browser transforms). Have you thought of using the Module.react.js convention? If we went with that, all the editors will not break.

Member

jordwalke commented Jun 20, 2013

👍

What are you thinking as a substitute? We should cover in-browser transforms as well as something like require.js (with or without in-browser transforms). Have you thought of using the Module.react.js convention? If we went with that, all the editors will not break.

@zpao

This comment has been minimized.

Show comment
Hide comment
@zpao

zpao Jun 20, 2013

Member

Well, we need to consider if we want JSX to be general purpose. As we talk about pulling it into it's own project we need to think about how it'll get used. If we want to keep with this idea that it's generic and you can plug in any "namespace", then we need to keep that working. Right now the transform takes @jsx Namespace and uses that to turn <div> into Namespace.div. But then we also have to make sure we support other transforms (e.g. React's displayName) that will be tool specific. Also, if I plug in a pure wrapper that does document.createElement, then I don't want to support custom components.

If we didn't have the concern of supporting other targets, then I would just say we'll transform anything that's type="text/jsx" and *.jsx on the command line (or *.react.js).

For the in-browser transform, we talked about doing something like <script src="file.js" type="text/jsx" data-jsx-namespace="React.DOM"></script>. Still doesn't quite solve the multiple transform possibilities without a map from namespace to transforms (React.DOM would use react and reactDisplayName).

For bin/jsx we could start accepting a list of transforms? If we assume JSX is a separate package, it ships the jsx executable. Then React would depend on JSX. React could ship the set of transforms it needs and maybe create a customized reactjsx executable that is basically just an alias to jsx --transform react --transform reactDisplayName $1 or whatever.

Member

zpao commented Jun 20, 2013

Well, we need to consider if we want JSX to be general purpose. As we talk about pulling it into it's own project we need to think about how it'll get used. If we want to keep with this idea that it's generic and you can plug in any "namespace", then we need to keep that working. Right now the transform takes @jsx Namespace and uses that to turn <div> into Namespace.div. But then we also have to make sure we support other transforms (e.g. React's displayName) that will be tool specific. Also, if I plug in a pure wrapper that does document.createElement, then I don't want to support custom components.

If we didn't have the concern of supporting other targets, then I would just say we'll transform anything that's type="text/jsx" and *.jsx on the command line (or *.react.js).

For the in-browser transform, we talked about doing something like <script src="file.js" type="text/jsx" data-jsx-namespace="React.DOM"></script>. Still doesn't quite solve the multiple transform possibilities without a map from namespace to transforms (React.DOM would use react and reactDisplayName).

For bin/jsx we could start accepting a list of transforms? If we assume JSX is a separate package, it ships the jsx executable. Then React would depend on JSX. React could ship the set of transforms it needs and maybe create a customized reactjsx executable that is basically just an alias to jsx --transform react --transform reactDisplayName $1 or whatever.

@ericclemmons

This comment has been minimized.

Show comment
Hide comment
@ericclemmons

ericclemmons Jul 1, 2013

Contributor

Would the noise be so bad if it didn't have to be first (or specially structured)?

/**
 * MyComponent
 *
 * This will render a super awesome component!
 *
 * @param object
 * @returns object
 * @jsx React.DOM
 */
var MyComponent = (function(React) {
  return React.createClass({ ... });
}(React);

Besides that, I've been using .jsx. I would assume anything explicitly passed to bin/jsx or the transform script wouldn't have to check for the docblock...

Contributor

ericclemmons commented Jul 1, 2013

Would the noise be so bad if it didn't have to be first (or specially structured)?

/**
 * MyComponent
 *
 * This will render a super awesome component!
 *
 * @param object
 * @returns object
 * @jsx React.DOM
 */
var MyComponent = (function(React) {
  return React.createClass({ ... });
}(React);

Besides that, I've been using .jsx. I would assume anything explicitly passed to bin/jsx or the transform script wouldn't have to check for the docblock...

@zpao

This comment has been minimized.

Show comment
Hide comment
@zpao

zpao Jul 1, 2013

Member

That long docblock should just work right now actually :) We aren't that strict about the format so long as it's in the first docblock in the file. I haven't tested but if that doesn't work, let me know.

And that assumption that anything passed to bin/jsx just works is a good one to make. But it doesn't actually work right now :/

Member

zpao commented Jul 1, 2013

That long docblock should just work right now actually :) We aren't that strict about the format so long as it's in the first docblock in the file. I haven't tested but if that doesn't work, let me know.

And that assumption that anything passed to bin/jsx just works is a good one to make. But it doesn't actually work right now :/

@sophiebits

This comment has been minimized.

Show comment
Hide comment
@sophiebits

sophiebits May 23, 2014

Member

Related to #832.

Member

sophiebits commented May 23, 2014

Related to #832.

@syranide

This comment has been minimized.

Show comment
Hide comment
@syranide

syranide May 23, 2014

Contributor

I'd say #1551 is kind of related too, if we go that way.

Contributor

syranide commented May 23, 2014

I'd say #1551 is kind of related too, if we go that way.

@syranide

This comment has been minimized.

Show comment
Hide comment
Contributor

syranide commented Oct 10, 2014

@syranide syranide closed this Oct 10, 2014

@fyyyyy

This comment has been minimized.

Show comment
Hide comment
@fyyyyy

fyyyyy Oct 31, 2014

this change really disturbed my jsx usage for mithril. have to see if the new version will work somehow

fyyyyy commented Oct 31, 2014

this change really disturbed my jsx usage for mithril. have to see if the new version will work somehow

@sophiebits

This comment has been minimized.

Show comment
Hide comment
@sophiebits

sophiebits Oct 31, 2014

Member

@fyyyyy You can continue to use react-tools 0.11 or make your own custom transformer, like https://github.com/Raynos/mercury-jsx.

Member

sophiebits commented Oct 31, 2014

@fyyyyy You can continue to use react-tools 0.11 or make your own custom transformer, like https://github.com/Raynos/mercury-jsx.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment