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

For #6 Support ES6 export declarations #12

Closed
wants to merge 3 commits into from
Closed

For #6 Support ES6 export declarations #12

wants to merge 3 commits into from

Conversation

morlay
Copy link
Contributor

@morlay morlay commented Apr 30, 2015

#6

import React from 'react';
export default React.createClass(...);
// or
export var Component = React.createClass(...);
// etc

This way is done.

Should we support this?

import React, { createClass } from 'react';
export default createClass(...);

And not sure, is it ok, to support ES6 module and CommonJS in one handler.

@facebook-github-bot
Copy link

Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla - and if you have received this in error or have any questions, please drop us a line at cla@fb.com. Thanks!

@facebook-github-bot
Copy link

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks!

@fkling
Copy link
Member

fkling commented Apr 30, 2015

Sweet! I will take a close look later today. Thank you for this!

@@ -47,6 +48,17 @@ function findExportedReactCreateClass(
visitDoWhileStatement: ignore,
visitForStatement: ignore,
visitForInStatement: ignore,
visitExportDeclaration: function(path) {
path = resolveToCallExpression(path);
Copy link
Member

Choose a reason for hiding this comment

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

Mmh, do we have to introduce resolveToCallExpression? My approach would have been to just extract the path from ExportDeclaration that we are interested in, and run resolveToValue on that.

Thoughts?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, seems we can change here with

path = resolveToValue(path.get('declaration'));

and add the conditions to resolveToValue

if (types.VariableDeclaration.check(node)) {
    return resolveToValue(path.get('declarations', 0))
  } else if (types.VariableDeclarator.check(node)) {
     if (node.init) {
       return resolveToValue(path.get('init'));
     }
  } 

I will push another commit to update the change.

however, I feel the resolveToValue need to do some refact when we added a lot of conditions.

Thanks.

@fkling
Copy link
Member

fkling commented May 1, 2015

It doesn't seem like this would support support

export {
  comeponent,
  something else
};

or does it?

Should we support this?

import React, { createClass } from 'react';
export default createClass(...);

Not sure. I think if the need arises we can still that.

And not sure, is it ok, to support ES6 module and CommonJS in one handler.

Seems fine to me.

@morlay
Copy link
Contributor Author

morlay commented May 2, 2015

Cool. I feel we are not necessary to write like that.

With react-docgen's help, we can get docs so easy, in my last projects.

So I put the docs tool in my testing repo (preview)

Recently, we need to update to ES6, that why I try to make the update for react-docgen.

@fkling
Copy link
Member

fkling commented May 18, 2015

Sorry for the late response! I included your changes in 55c1511 and 9e8670c (I squashed the third one).

I also built on top of your changes and added support for the remaining import and export variations.

All these changes are release with v1.3.0.

Thanks for your help!

@fkling fkling closed this May 18, 2015
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 29, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants