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

feat: add updateProps config option #868

Merged
merged 7 commits into from
Mar 21, 2018

Conversation

ryanoglesby08
Copy link
Contributor

I am working on a project using react-styleguidist where all the components are versioned npm packages. I don't like that the component version is duplicated in both the package.json and the component file, which is causing me to have to manually update it in the component file when I do a release (or forget to do that). It would be more maintainable to load the version from the package.json file so that they are always in sync.

This PR is a WIP to demonstrate this.

Would like some feedback on it and then can clean it up and build it out as a full solution.

@ryanoglesby08
Copy link
Contributor Author

And of course a more complete solution would have to check it the version is a path or if its a number, among other things :)

@sapegin
Copy link
Member

sapegin commented Mar 13, 2018

I'd accept a more generic solution: a function to process an object returned by getProps function. So you'll read package.json in your style guide config.

@codecov-io
Copy link

codecov-io commented Mar 13, 2018

Codecov Report

Merging #868 into master will increase coverage by 0.65%.
The diff coverage is 100%.

Impacted Files Coverage Δ
loaders/props-loader.js 97.82% <100%> (+0.68%) ⬆️
scripts/schemas/config.js 86.2% <100%> (ø) ⬆️
loaders/styleguide-loader.js 100% <0%> (ø) ⬆️
src/utils/getRouteData.js 100% <0%> (ø) ⬆️
.../rsg-components/TableOfContents/TableOfContents.js 94.11% <0%> (ø) ⬆️
src/rsg-components/slots/index.js 100% <0%> (ø) ⬆️
...rc/rsg-components/StyleGuide/StyleGuideRenderer.js 100% <0%> (ø) ⬆️
src/styles/theme.js 100% <0%> (ø) ⬆️
loaders/utils/getComponentFiles.js 100% <0%> (ø) ⬆️
... and 13 more

@ryanoglesby08
Copy link
Contributor Author

Ok, attempt 2.

I've introduced a new config option called propsPostProcessor of type function. If it exists it will get invoked after loading and initial processing of the props.

This shifts the responsibility for loading the version from a file to the user.

What do you think?

@@ -131,6 +131,9 @@ module.exports = {
propsParser: {
type: 'function',
},
propsPostProcessor: {
Copy link
Member

Choose a reason for hiding this comment

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

updateProps would be a better name to match updateExample: https://react-styleguidist.js.org/docs/configuration#updateexample

module.exports = {
components: 'src/components/**/[A-Z]*.js',
defaultExample: true,
propsPostProcessor(props, file) {
Copy link
Member

Choose a reason for hiding this comment

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

Can we move this to another example styleguide? I'd like to use basic example for the core functionality.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed the example and instead included documentation for updateProps

Copy link
Member

Choose a reason for hiding this comment

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

I think you forget to push some changes: the example is still here ;-/

module.exports = {
components: 'src/components/**/[A-Z]*.js',
defaultExample: true,
propsPostProcessor(props, file) {
// If there is a filename for the version, then load it in.
if (props.doclets.version) {
Copy link
Member

Choose a reason for hiding this comment

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

More realistic example would be: check if @version has a value: if it has, use it, otherwise read version from version.json.

Copy link
Contributor Author

@ryanoglesby08 ryanoglesby08 Mar 15, 2018

Choose a reason for hiding this comment

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

I can make the sample code in the docs more complex if you like.... but I feel that it won't add anything since its just sample code in the docs.

Copy link
Member

Choose a reason for hiding this comment

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

Good enough now ;-)

* `updateProps` can be used to modify the component props after parsing
them out of the source file
* Use object spread syntax in tests to prevent test conflicts due to
modification of the test input object with `Object.assign`
* Add documentation for the new config option
@ryanoglesby08 ryanoglesby08 changed the title feat: read component version from an external file WIP feat: add updateProps config option Mar 15, 2018
@ryanoglesby08 ryanoglesby08 changed the title feat: add updateProps config option feat: add updateProps config option Mar 15, 2018
@sapegin sapegin mentioned this pull request Mar 19, 2018
5 tasks
@ryanoglesby08
Copy link
Contributor Author

Hi @sapegin any more feedback here? I've reverted the old code here and Travis CI is passing now.

Copy link
Member

@sapegin sapegin left a comment

Choose a reason for hiding this comment

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

Looks good! 🍕

const versionFilePath = path.resolve(path.dirname(file), props.doclets.version);
const version = require(versionFilePath).version;

props.doclets.version = version;
Copy link
Member

Choose a reason for hiding this comment

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

I believe this line is unnecessary but worth checking.

module.exports = {
components: 'src/components/**/[A-Z]*.js',
defaultExample: true,
propsPostProcessor(props, file) {
Copy link
Member

Choose a reason for hiding this comment

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

I think you forget to push some changes: the example is still here ;-/

module.exports = {
components: 'src/components/**/[A-Z]*.js',
defaultExample: true,
propsPostProcessor(props, file) {
// If there is a filename for the version, then load it in.
if (props.doclets.version) {
Copy link
Member

Choose a reason for hiding this comment

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

Good enough now ;-)

@sapegin sapegin merged commit b444d11 into styleguidist:master Mar 21, 2018
@sapegin sapegin added this to the 6.3.0 milestone Mar 21, 2018
@ryanoglesby08 ryanoglesby08 deleted the read-version-from-file branch March 21, 2018 18:29
sapegin pushed a commit that referenced this pull request Mar 22, 2018
👋 **[Support Styleguidist](https://opencollective.com/styleguidist) on Open Collective** 👋

## New features

### Page per section 🚧

This is a huge improvement for large style guides: now you can show only one component at a page instead of all components.

![](https://d3vv6lp55qjaqc.cloudfront.net/items/0W2q2K2s3n2k311O1J0y/Screen%20Recording%202018-03-22%20at%2010.06%20AM.gif)

To enable:

```js
module.exports = {
  pagePerSection: true
};
```

This is an experimental feature and we need your feedback to make it better and really useful. For example, right now there’s no isolated mode. So feel free to share your ideas [in issues](https://github.com/styleguidist/react-styleguidist/issues).

(#835 by @bitionaire and @stepancar, closes #494 and #768)

### “Fork me” ribbon

One more step to make Styleguidist usable for documenting open source projects:

![](https://d3vv6lp55qjaqc.cloudfront.net/items/1t331n2a3v0F2i290K0Z/Image%202018-03-22%20at%209.13.11%20AM.png)

New config option to enable the ribbon, define the URL and change the label:

```js
module.exports = {
  ribbon: {
    url: 'http://example.com/',
    text: 'Fork me on GitHub'
  }
};
```

And two new [theme variables](https://github.com/styleguidist/react-styleguidist/blob/master/src/styles/theme.js) to change colors: `color.ribbonBackground` and `color.ribbonText`.

(#861 by @glebez and @wkwiatek, part of #195, closes #647)

### Options to change CLI output on server start and build

Two new options, `printServerInstructions` and `printBuildInstructions`:

```js
module.exports = {
  printBuildInstructions(config) {
    console.log(
      `Style guide published to ${
        config.styleguideDir
      }. Something else interesting.`
    );
  }
};
```

(#878 by @roblevintennis, closes #876)

### Option to modify props

A new option, `updateDocs` to modify props before rendering. For example, you can load a component version number from a JSON file:

```js
module.exports = {
  updateDocs(docs) {
    if (docs.doclets.version) {
      const versionFilePath = path.resolve(
        path.dirname(file),
        docs.doclets.version
      );
      const version = require(versionFilePath).version;

      docs.doclets.version = version;
      docs.tags.version[0].description = version;
    }

    return docs;
  }
};
```

(#868 by @ryanoglesby08)

### Limited support for named exports

Styleguidist used to require default exports for components. Now you can use named exports too, though Styleguidist still supports only one component per file. I hope this change will make some users happier, see more details [in the docs](https://react-styleguidist.js.org/docs/components.html#loading-and-exposing-components).

(#825 by @marcdavi-es, closes #820 and #633)

### Allow arrays of component paths in sections

More flexibility in structuring your style guide:

```js
module.exports = {
  sections: [
    {
      name: 'Forms',
      components: [
        'lib/components/ui/Button.js',
        'lib/components/ui/Input.js'
      ]
    }
  ]
};
```

(#794 by @glebez, closes #774)

### Sort properties by `required` and `name` attributes

This change should make props tables more predictable for the users. Instead of relying on developers to sort props in a meaningful way, Styleguidist will show required props first, and sort props in each group alphabetically.

To disable sorting, use the identity function:

```javascript
module.exports = {
  sortProps: props => props
};
```

(#784 by @dotcs)

## Bug fixes

* Allow `Immutable` state in examples (#870 by @nicoffee, closes #864)
* Change template container ID to prevent clashes (#859 by @glebez, closes #753)
* Better props definitions with Flow types (#781 by @nanot1m)
@sapegin
Copy link
Member

sapegin commented Mar 22, 2018

🎉 This PR is included in version 6.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

3 participants