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

Better support and tools for multiple exported components per file #283

Closed
wants to merge 4 commits into from

Conversation

madjam002
Copy link

This is more of an experiment but I'd like to know what your thoughts are on this.

There's a few things going on in this PR:

  • Fix multiple components from same file causing a duplicate React keys error
  • Allow for import statements in examples and for importing components
  • Allow for adjacent JSX by wrapping example JSX in an enclosing <div />
  • Add ability to import @example markdown by section

A few notes on a couple of things mentioned above:

Allow for import statements in examples

When you export named components from a module, Styleguidist will require you to reference them within the module e.g

// Header.jsx

export const H1 = props => (...)
export const H2 = props => (...)
// Example
<Header.H1>Hello, world</Header.H1>

By allowing the use of import, the example can have an example of how the component might actually be imported in real world usage, and of course it is then able to correctly locate the component:

// Example
import {H1} from 'components';

<H1>Hello, world!</H1>

Unfortunately, as you can see in the PR, the way I have done this is with some major hackery. I am more interested however with whether you think this is a good idea, and if so we can then look into refactoring.

Ability to import @example by section

Just like wanting to have multiple small components per file, it might be desirable to have all of the documentation for these components in one file as well. This is difficult because if you create a Readme.md, it will be displayed for every component in the single file.

I've solved this by taking advantage of the existing ability to import an external example with @example, but now you can reference a section/anchor in that markdown file.

e.g:

// Readme.md

This is an example README.

# H1

This is the documentation for `<H1 />`.

    <H1>Hello, world!</H1>

# H2

You get the idea...
// Header.jsx

/** @example ./Readme.md#H1 */
export const H1 = props => (...)

/** @example ./Readme.md#H2 */
export const H2 = props => (...)

This enables all of the documentation for relevant components to be in one place, which is great for small components with a minimal API.

Let me know what your thoughts are, thanks!

This fixes only one component appearing when using
findAllExportedComponentDefinitions due to the duplicate React keys
error.
For example:
/**
  * @example ./Readme.md#MyComponent
 */

This allows for multiple components to be documented in a single
markdown file and be placed next to the appropriate components in
the styleguide.
@sapegin
Copy link
Member

sapegin commented Jan 11, 2017

Thank you very much for your pull request! This all very interesting and I’m sure useful.

Would you mind to send a new PR to the next branch? It’s a new major release which is almost ready (code and docs are 95% done and more testing is needed), see #223 for more details. The reason I’m asking you to do a lot more work is: every big pull request will delay new release more since I’ll have to port it to the new branch, and code base is very different in some parts (especially loaders). Probably even better to split this into multiple PRs to make it easier to review and merge.

Feel free to ping me in Gitter (or by email/Facebook/Skype) if you have any questions.

@madjam002
Copy link
Author

Sounds good, I'll also split it into multiple PRs. Thanks 😃

@sapegin
Copy link
Member

sapegin commented Jan 11, 2017

@madjam002 Thank you for understanding! 🤝

@madjam002
Copy link
Author

Just FYI - I'll just wait for next to be merged into master as we're happy using my fork internally for now and it would be great to get some of these ideas explored further once next is merged.

@sapegin
Copy link
Member

sapegin commented Jan 23, 2017

@madjam002 Sounds good!

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.

2 participants