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

#114 - stop requiring docblock for JSX transformer #418

Closed
wants to merge 2 commits into from

Conversation

SanderSpies
Copy link
Contributor

Revisiting #364 #114

(sorry for the extra pull request - used Github wrong the last time)

@SanderSpies
Copy link
Contributor Author

Incomplete.

@SanderSpies SanderSpies reopened this Oct 10, 2013
@jordwalke
Copy link
Contributor

👍

@fabiomcosta
Copy link
Contributor

@SanderSpies it seems like this PR is outdated and is causing some simple merge conflicts (getDocblock is now utils. getDocblock).
Also please use ' instead of " as React's styleguide points out: https://github.com/facebook/react/blob/master/CONTRIBUTING.md#coding-style

@@ -180,7 +180,7 @@ function visitReactTag(traverse, object, path, state) {
visitReactTag.test = function(object, path, state) {
// only run react when react @jsx namespace is specified in docblock
var jsx = getDocblock(state).jsx;
return object.type === Syntax.XJSElement && jsx && jsx.length;
return object.type === Syntax.XJSElement && (jsx === undefined || (jsx && jsx.length && jsx === "React.DOM"));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this could be simplified to return object.type === Syntax.XJSElement && (jsx == null || jsx === 'React.DOM');

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I believe that from this comment of @jeffmo #364 (comment) he actually meant return object.type === Syntax.XJSElement && (!jsx || jsx === 'React.DOM');

@vjeux
Copy link
Contributor

vjeux commented Dec 28, 2013

What's the status here. Do we want to go forward with this change?

@vjeux
Copy link
Contributor

vjeux commented Dec 31, 2013

This PR is 3 months old. I'm going to close it out. Feel free to re-open the issue.

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.

4 participants