Add limited support for named exports. Add docs relating to this support. #825

marcdavi-es · 2018-02-15T16:48:14Z

Summary

This responds to the need expressed in #820 and #633

It preserves the base case of Styleguidist loading default exports by default, while adding some limited support for exposing named exports where a module exposes no default export and either:

just one named export; or
a named export which matches the component name understood by Styleguidist.

I think these cases are non-controversial. In each case, it is almost guaranteed that the developer intends these named exports to be interpreted as the exported component.

I have added documentation covering this. The documentation is careful to emphasise that this is not a complete solution for named exports - Styleguidist will do its best but in complicated cases will just fall back to exposing the module as a whole, as it currently does.

Super happy to take questions and feedback on this. I love what you guys are doing with this tool.

Question

In the existing implementation of globalizeComponent, there are references to component.props.path. I couldn't get this property to show its face with a bunch of different configurations/attempts. I suspect maybe it's a legacy thing which is still hanging around.

If I'm missing something here please let me know and I'll take account of this in the PR.

codecov-io · 2018-02-16T09:28:32Z

Codecov Report

Merging #825 into master will increase coverage by 0.16%.
The diff coverage is 100%.

Impacted Files	Coverage Δ
src/utils/getComponent.js	`100% <100%> (ø)`
src/utils/globalizeComponent.js	`100% <100%> (+28.57%)`	⬆️
src/rsg-components/Markdown/Markdown.js	`100% <0%> (ø)`	⬆️
...g-components/Markdown/Checkbox/CheckboxRenderer.js	`100% <0%> (ø)`

…ined component name

marcdavi-es · 2018-02-16T13:53:53Z

One of the tests fails on one of the builds but passes on the others. It seems that the afterEach block in globalizeComponent.spec.js isn't doing its job in this case.

I tried refactoring the test to avoid reliance on the enumerable properties of global but still no joy. Would appreciate any input on this.

sapegin

Thanks, this is interesting and I hope will make Styleguidist more flexible.

In the existing implementation of globalizeComponent, there are references to component.props.path.

I think it's safe to remove if everything still works.

I tried refactoring the test to avoid reliance on the enumerable properties of global but still no joy. Would appreciate any input on this.

Maybe try to put afterEach inside describe? Just a guess ;-/

sapegin · 2018-02-16T19:32:35Z

docs/Components.md

+// will fall back on the file name, convert it to PascalCase
+// and expose globally as ComponentFour
+```
+


There's one more use case: folder name when the file name is index.js.

sapegin · 2018-02-16T19:32:45Z

docs/Components.md

+// and expose globally as ComponentFour
+```
+
+### default vs named exports


sapegin · 2018-02-16T19:37:39Z

docs/Components.md

+
+### default vs named exports
+
+Styleguidist defaults to exposing the `default` export from your component file. It understands functions exported as CommonJs `module.exports` as default exports.


Maybe “Stylegudist will use ECMAScript module’s default export or CommonJS module.exports if they are defined”?

sapegin · 2018-02-16T19:38:43Z

docs/Components.md

+// will be exposed globally as ComponentEight
+```
+
+If you export several React components as named exports from a single module, Styleguidist is likely to behave unreliably. If it cannot understand which named export to expose, it will fall back to exposing the module as a whole.


it will fall back to exposing the module as a whole.

Not sure what exactly that mean, but most likey you'll not be able to access components, so we can say that instead ;-)

sapegin · 2018-02-16T19:40:46Z

docs/Components.md

+
+Styleguidist _loads_ your components and _exposes_ them globally for your examples to consume.
+
+### Identifier


Would be nice to make this section shorted and easier to scan. Maybe use one big table instead?

Resorting to HTML table format here to play nicely with the code blocks. It makes this file a little ugly but the output is good.

Could be simpler:

| foo | bar | | --- | --- | | `foo`<br>`bar` | bar |

Also you can remove the last column and use the same component name everywhere, and merge displayName and fallback columns since they are mutually exclusive.

Ah yeah nice. Done.

Wow, it's like 10 times smaller now ;-)

sapegin · 2018-02-16T19:43:59Z

src/utils/getComponent.js

+ * @param  {string} name
+ * @return {function|object}
+ */
+export default function getComponent(module, name) {


I guess your understanding of this code is very good now, could you please add comments to each variation? Preferably with code examples. This is one of the most obscure parts of the code base ;-/

Nice! Please add a blank line before each comment to make it easier to read.

sapegin · 2018-02-16T19:47:27Z

src/utils/__tests__/globalizeComponent.spec.js

@@ -1,18 +1,30 @@
 import globalizeComponent from '../globalizeComponent';

+jest.mock('../getComponent');


Can we avoid mocking here? Doesn't look necessary.

sapegin · 2018-02-16T19:48:29Z

src/utils/__tests__/globalizeComponent.spec.js

@@ -1,18 +1,30 @@
 import globalizeComponent from '../globalizeComponent';

+jest.mock('../getComponent');
+// eslint-disable-next-line import/first
+import getComponent, { returnValue } from '../getComponent';


ESM imports are hoisted to the top of the file, so disabling ESLint here is unnecessary and misleading.

marcdavi-es · 2018-02-16T20:40:48Z

Thanks for the feedback :)

I'm out this weekend but I'll get to this on Monday.

marcdavi-es · 2018-02-19T11:46:58Z

@sapegin I think that's everything. Let me know if you're happy with those changes :)

sapegin

Thanks, almost!

sapegin · 2018-02-19T12:44:07Z

docs/Components.md

+
+Styleguidist _loads_ your components and _exposes_ them globally for your examples to consume.
+
+### Identifier


Could be simpler:

| foo | bar | | --- | --- | | `foo`<br>`bar` | bar |

Also you can remove the last column and use the same component name everywhere, and merge displayName and fallback columns since they are mutually exclusive.

sapegin · 2018-02-19T12:45:41Z

src/utils/getComponent.js

+ * @param  {string} name
+ * @return {function|object}
+ */
+export default function getComponent(module, name) {


Nice! Please add a blank line before each comment to make it easier to read.

marcdavi-es · 2018-02-19T16:35:03Z

src/utils/getComponent.js

+ * @return {function|object}
+ */
+export default function getComponent(module, name) {
+	//


Had to leave this little 'comment' in here to stop Prettier complaining about the blank line...

Sorry, I wasn't clear enough: blank lines to separate code “paragraphs”, not just empty line before each comment ;-)

So you mean changing this

// If the module defines a default export, return that // e.g. // ``` // export default function Component() { ... } // ``` if (module.default) { ...

to this

// If the module defines a default export, return that // e.g. // // ``` // export default function Component() { ... } // ``` // if (module.default) { ...

Or something else? :)

Exactly! Just to separate different sections of code.

marcdavi-es · 2018-02-19T16:36:18Z

@sapegin Thanks - yeah that table was ugly. Think it's much cleaner now.

sapegin · 2018-02-19T17:28:38Z

docs/Components.md

+
+Styleguidist _loads_ your components and _exposes_ them globally for your examples to consume.
+
+### Identifier


Wow, it's like 10 times smaller now ;-)

marcdavi-es · 2018-02-20T09:06:20Z

@sapegin Sweet - comments spaced out better now :)

@bitionaire

👋 **[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 · 2018-03-22T10:26:36Z

🎉 This PR is included in version 6.5.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Marc added 3 commits February 15, 2018 13:32

Globalise named export if no default export present in component module.

59b1a71

Add documentation for identifier and default vs named exports

69563d6

Add case for CommonJS module

9937186

marcdavi-es mentioned this pull request Feb 15, 2018

Support ES6 exports #820

Closed

marcdavi-es closed this Feb 16, 2018

marcdavi-es reopened this Feb 16, 2018

Change test case to avoid unreliable result in CI. Add case for undef…

5f369bd

…ined component name

sapegin requested changes Feb 16, 2018

View reviewed changes

Marc added 3 commits February 19, 2018 12:24

Respond to PR comments on Components.md

d3c309d

Add comments to getComponent.js. Remove mocking on tests.

dcdee30

Remove superfluous test case

5e987e6

sapegin requested changes Feb 19, 2018

View reviewed changes

Marc added 2 commits February 19, 2018 17:29

Simplify identifier table

a3fde51

Make comments more readable

9e07efc

marcdavi-es commented Feb 19, 2018

View reviewed changes

sapegin requested changes Feb 19, 2018

View reviewed changes

Prettify comment blocks

90b998f

sapegin approved these changes Feb 21, 2018

View reviewed changes

sapegin added this to the 6.3.0 milestone Feb 21, 2018

sapegin merged commit fb1d2ec into styleguidist:master Mar 21, 2018

This was referenced Jul 17, 2020

[Snyk] Security upgrade react-styleguidist from 6.0.33 to 6.5.0 Bhanditz/react-styleguidist#46

Open

[Snyk] Fix for 1 vulnerabilities Bhanditz/react-styleguidist#47

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add limited support for named exports. Add docs relating to this support. #825

Add limited support for named exports. Add docs relating to this support. #825

marcdavi-es commented Feb 15, 2018

codecov-io commented Feb 16, 2018 •

edited

Loading

marcdavi-es commented Feb 16, 2018

sapegin left a comment

sapegin Feb 16, 2018

sapegin Feb 16, 2018

sapegin Feb 16, 2018

sapegin Feb 16, 2018

sapegin Feb 16, 2018

marcdavi-es Feb 19, 2018

sapegin Feb 19, 2018

marcdavi-es Feb 19, 2018

sapegin Feb 19, 2018

sapegin Feb 16, 2018

sapegin Feb 19, 2018

sapegin Feb 16, 2018

sapegin Feb 16, 2018

marcdavi-es commented Feb 16, 2018

marcdavi-es commented Feb 19, 2018

sapegin left a comment

sapegin Feb 19, 2018

sapegin Feb 19, 2018

marcdavi-es Feb 19, 2018

sapegin Feb 19, 2018

marcdavi-es Feb 19, 2018

sapegin Feb 19, 2018

marcdavi-es commented Feb 19, 2018

sapegin Feb 19, 2018

marcdavi-es commented Feb 20, 2018

sapegin commented Mar 22, 2018


		### default vs named exports

		Styleguidist defaults to exposing the `default` export from your component file. It understands functions exported as CommonJs `module.exports` as default exports.


		Styleguidist _loads_ your components and _exposes_ them globally for your examples to consume.

		### Identifier

		@@ -1,18 +1,30 @@
		import globalizeComponent from '../globalizeComponent';

		jest.mock('../getComponent');

Add limited support for named exports. Add docs relating to this support. #825

Add limited support for named exports. Add docs relating to this support. #825

Conversation

marcdavi-es commented Feb 15, 2018

Summary

Question

codecov-io commented Feb 16, 2018 • edited Loading

Codecov Report

marcdavi-es commented Feb 16, 2018

sapegin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcdavi-es commented Feb 16, 2018

marcdavi-es commented Feb 19, 2018

sapegin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marcdavi-es commented Feb 19, 2018

Choose a reason for hiding this comment

marcdavi-es commented Feb 20, 2018

sapegin commented Mar 22, 2018

codecov-io commented Feb 16, 2018 •

edited

Loading