-
Notifications
You must be signed in to change notification settings - Fork 47.2k
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
Solves issue #114 - Stop requiring docblock for JSX transformer #364
Conversation
SanderSpies
commented
Sep 21, 2013
- '@jsx something' is still supported
- by default React.DOM is used (so no @jsx is needed)
@@ -56,7 +56,7 @@ var JSX_ATTRIBUTE_TRANSFORMS = { | |||
}; | |||
|
|||
function visitReactTag(traverse, object, path, state) { | |||
var jsxObjIdent = getDocblock(state).jsx; | |||
var jsxObjIdent = getDocblock(state).jsx || "React.DOM"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm going to let @jeffmo take a look at this. We had talked about some other ways to do it so I think we should solidify a plan with him before taking it further. One thing we actually got rid of before open sourcing was this default namespace. I think we should stick with that and instead make it possible for the tools (eg bin/jsx
, vendor/browser-transforms.js
) to specify the namespace. In React, they would specify React.DOM
but some other library may want something else.
I'd really like to see this parameter passed in at the transformer level eventually (and not specified in the docblock altogether). However, this particular transform (transforms/react.js) is very much react-specific though...so I can't think of any issues with it assuming that it needs to use 'React.DOM' as the default case. Let me patch this locally and play with it to see if I can come up with any edge-cases though before I accept. |
@@ -177,10 +177,8 @@ function visitReactTag(traverse, object, path, state) { | |||
return false; | |||
} | |||
|
|||
visitReactTag.test = function(object, path, state) { | |||
// only run react when react @jsx namespace is specified in docblock | |||
var jsx = getDocblock(state).jsx; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should still check for the '@jsx' flag in the docblock here and pass the test only if the attribute value is empty or if the attribute value is 'React.DOM'
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry -- that should have said "if the attribute value is empty or 'React.DOM'" (i.e. not null
)
Any further discussion should go in #418. |
…k#364) * Refactor inspect/select logic so that var contains hooks data * Legacy renderer resets $r to null when inspecting non class/function element