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

Add hooks to enable multiple components per file #106

Merged
merged 5 commits into from
Mar 28, 2016

Conversation

paulj
Copy link
Collaborator

@paulj paulj commented Mar 23, 2016

In structuring our application, we often have a number of exports from individual files, since the components are often via tightly linked (for instance, I have a Columns component with a Column child type).

I'd like to be able to use styleguidist to document these. React docgen can be convinced to emit the metadata for these, but the resolver needs to be overriden. This PR adds the ability to configure the react docgen resolver and handlers via the styleguidist configuration file. It also adds some (initially sensible) behaviour for dealing with an array of components being returned in props instead of just a flat set of props.

For reference, with this patch, I'm configuring styleguidist like:

resolver: require('react-docgen').resolver.findAllComponentDefinitions,
handlers: require('react-docgen').defaultHandlers.concat(function(documentation, path) {
    if (path.value.type == 'ClassDeclaration' && path.value.id.type == 'Identifier') {
      documentation.set('displayName', path.value.id.name);

      if (path.parentPath.value.type == 'ExportNamedDeclaration') {
        documentation.set('path', path.value.id.name);  
      }
    }

    if (path.parentPath.value.type == 'ExportDefaultDeclaration') {
      documentation.set('path', 'default');
    }
  }

This results in all components being found and props being generated. The additional handler attaches a displayName and path property to items, enabling the name of the component to be detected. The displayName property is similar to a default behaviour built-in to react-docgen where a React.createClass declaration displayName is carried through in the props.

@sapegin
Copy link
Member

sapegin commented Mar 23, 2016

Thanks! Could you please update the docs as well?

@paulj
Copy link
Collaborator Author

paulj commented Mar 23, 2016

I've added documentation for the resolver and handlers properties into the main Readme. Is there anywhere else that needs to be updated?

},
{
module: {displayName: 'Foo'},
props: {displayName: 'FooOverride'}
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand what does props mean in this context.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Props would be the object generated by the props loader. The displayName property can be injected by a react-docgen handler - either the built in one, or a custom one like I suggest in the docs.

@mik01aj
Copy link
Collaborator

mik01aj commented Mar 23, 2016

@paulj how will this change affect existing styleguides? How does it integrate with the getExampleFilename config option, and how are the examples then displayed (under which component)?

@paulj
Copy link
Collaborator Author

paulj commented Mar 23, 2016

@mik01aj this change should have no effect on existing guides, since if the options aren't specified, the behaviours default to the same as previously.

For the examples, they are currently allocated to all components in the file, since spreading the props out is done by duplicating all the other collected data. This isn't ideal - I'm investigating some other ways of targeting examples.

@paulj
Copy link
Collaborator Author

paulj commented Mar 28, 2016

@mik01aj with #109 merged, examples can now be annotated onto specific components with the @example doclet, which allows files with multiple examples to use that syntax instead of creating a file-wide examples file.

@sapegin
Copy link
Member

sapegin commented Mar 28, 2016

Should I merge this too?

@paulj
Copy link
Collaborator Author

paulj commented Mar 28, 2016

@sapegin I've just merged master in, and applied some other stylistic fixes based upon your comments on other PRs.

@sapegin sapegin merged commit 3a7271f into styleguidist:master Mar 28, 2016
@chrisdrackett
Copy link

chrisdrackett commented Nov 4, 2016

@paulj I have a file with about 5 components in it that I'm trying to get working. I have all the components rendering in the sidebar, but only the first is shown in the style guide itself. Any thoughts as to what might be going on?

looks like something with keys Encountered two children with the same key I'll keep digging.

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.

None yet

4 participants