Components: Remove PropsTable, clean ArgsTable stories #11105

shilman · 2020-06-09T21:22:47Z

Issue: N/A

What I did

This is pre-work for an ArgsTable redesign

How to test

CI is passing

github-actions · 2020-06-09T21:24:48Z

	Fails
🚫	PR is marked with "BREAKING CHANGE" label.

Generated by 🚫 dangerJS against 51e94df

shilman · 2020-06-10T02:09:55Z

@tmeasday this is some pre-work for @domyen 's updated ArgsTable design. Can you quickly review the changes in *.stories.tsx before I move on to the main event? If you're good with the changes, I will do a blanket update to the storybook repo with this kind of stuff, as part of the 6.0 args push.

tmeasday

Stories look great a couple suggestions

lib/components/src/blocks/ArgsTable/ArgsTable.stories.tsx

tmeasday · 2020-06-10T02:14:36Z

lib/components/src/blocks/DocsPage.stories.tsx

-    {propsTableStories.normal()}
-    {sourceStories.jsx()}
-  </>
+    <Description.Text {...Description.Text.args} />


Maybe you should do the default parameter binding trick for these stories?

The default parameter binding trick doesn't seem to work. <Description.Text /> seems to fill in props with {}, overriding the trick

Yeah I see what you mean, I guess react passes {} by default not undefined which breaks that pattern.

So for react it's really Story.defaultProps = Story.args method the only way?

Bruno-Tavares-Bose · 2020-06-15T16:33:26Z

I know that this is a beta version and this PR is marked with BREAKING CHANGE, but, since this (beta.23 is ok), I can not see props in our React components. We are using MDX (docs addon), and our Vue components have their props in there. Is there any readme, to understand the breaking changes? And why the Vue components are ok, but not the React ones?

Thanks in advance.

[edited] I can not see

shilman · 2020-06-16T00:19:36Z

We try to document all breaking changes here: https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#from-version-53x-to-60x

This PR in particular is a breaking change because I removed a component from @storybook/components, so any code that used that component would break. I'm pretty sure only Storybook used that component internally, but since it's a public API I don't know.

Bruno-Tavares-Bose · 2020-06-16T14:03:21Z

Thanks for the fast reply.

From the docs and code, I don't get why we are not seeing our props in React components.

We have one storybook for both React and Vue components, and we are doing these:

import { extractArgTypes as reactExtractArgTypes } from '@storybook/addon-docs/dist/frameworks/react/extractArgTypes';
import { extractArgTypes as vueExtractArgTypes } from '@storybook/addon-docs/dist/frameworks/vue/extractArgTypes';
import { createElement } from 'react';
import toReact from '@egoist/vue-to-react';

export const parameters = {
	docs: {
		extractArgTypes(component) { // would be awesome to have access to context like we have in decorators 😉
			if (typeof component === 'function') {
				return reactExtractArgTypes(component);
			}

			return vueExtractArgTypes(component);
		}
	}
};

export const decorators = [
	(storyFn, { kind }) => {
		let story = storyFn();

		const [framework] = kind.split('/');
		switch (framework) {
			case 'vue':
				const Story = toReact(story);
				story = createElement(Story, null);
				break;
		}

		return story;
	}
];

This have been working till beta.23, but after beta.24 we are getting the Vue as normal, but not the React. I already tried to comment the extractArgTypes method, and we get the Vue props (but the react table [without props title row for example]), but never the React.
We are using React functions components with hooks (no typescript).

Putting a console.log in estractArgTypes, it seems that the React component don't have the __docgenInfo. For Vue to work with React we also add the:

module: {
	rules: [
		{
			test: /\.vue$/,
			loader: 'vue-docgen-loader',
			enforce: 'post',
			options: {
				docgenOptions: Object.assign(
					{
						alias: config.resolve.alias
					},
					options.vueDocgenOptions,
					{
						alias: resolve.alias
					}
				)
			}
		}
	]
}

in main.js, should we have to use the a react docgen loader? And how it would be?

Again, thanks in advance

shilman · 2020-06-16T14:45:42Z

@Bruno-Tavares-Bose

First off, the way you are using Storybook is definitely not supported. We are releasing a new feature called composition in SB 6.0, that should allow you to mix frameworks more cleanly.

That said, the big change in beta.24 that's affecting your props generation is probably this one: #11106

There's one open issue so far that might be related: #11146

And there's an open PR that attempts to fix it: #11184

Hope that helps! cc @hipstersmoothie

Bruno-Tavares-Bose · 2020-06-16T15:45:16Z

Exploring a bit this issue, I also ended up in #11106 . Forcing the babel-plugin-react-docgen (bypassing this) fix the props (we are receiving some warnings now the types mismatch, picking).
And passing this typescript: { reactDocgen: 'react-docgen' } in main.js have the same result.

I will follow that PR. Thanks for the help, and continue with the awesome work 🙇

hipstersmoothie · 2020-06-16T16:18:04Z

We are using React functions components with hooks (no typescript).

This stands out to me. I don' t think storybook should be using react-docgen-typescript-plugin when it isn't typescript. react-docgen-typescript works by doing the following:

loads code into typescript
extracts docgen info from the compiler

If a user is using prop-types instead of interfaces the docgen from react-docgen-typescript-plugin will produce nothing.

hipstersmoothie · 2020-06-16T16:24:19Z

Thinking about this I should probably remove this check https://github.com/storybookjs/storybook/blob/next/app/react/src/server/framework-preset-react-docgen.ts#L9-L11

so that babel-docgen is always loaded. Since the default is react-docgen-typescript a JS-only user would always have to set reactDocgen to react-docgen.

@shilman @mrmckeb I feel like there should be some way to detect if we want the babel-plugin-react-docgen plugin to be loaded rather than always defaulting to typescript

EDIT: but then there isn't a guarantee that there isn't a mix of JS and TS components...

Bruno-Tavares-Bose · 2020-06-16T20:26:58Z

Hi @hipstersmoothie I think that is exactly the issue.

And sorry to spam this PR 😊

hipstersmoothie · 2020-06-16T20:29:04Z

It's kinda bad but I think storybook will have to do the docgen parsing twice because there is no way we can detect if a project is:

pure JS
pure TS
mixed JS/TS

hipstersmoothie · 2020-06-16T20:31:09Z

It might make more sense for react-docgen to be the default and users can opt-in to react-docgen-typescript if they want more detailed prop docs.

@shilman thoughts?

shilman · 2020-06-17T00:16:01Z

@hipstersmoothie we tried making react-docgen the default already (for performance reasons) but its typescript handling is just not good enough yet. I don’t understand the issue? Why can’t react-docgen-typescript ONLY apply to .ts/.tsx files? I think we had that setup working before with react-docgen-typescript-loader cc @mrmckeb

hipstersmoothie · 2020-06-17T00:26:24Z

The problem is this line which makes it so that 'babel-plugin-react-docgen' is only loaded when it is set to use that.

This means that JS only users do not have 'babel-plugin-react-docgen' loaded because the default is react-docgen-typescript.

I changed that line because I didn't want 2 docgen to run on my components: 1 by babel and 1 by the plugin.

shilman · 2020-06-17T00:37:35Z

what about this line:

https://github.com/storybookjs/storybook/blob/next/app/react/src/server/framework-preset-react-docgen.ts#L17

doesn't it skip tsx? files if user is running react-docgen-typescript? i think the line you mention is just a bug.

hipstersmoothie · 2020-06-17T01:45:35Z

Totally missed that. You’re right!

Bruno-Tavares-Bose · 2020-06-18T16:03:51Z

Great discussion guys. Just tried the new release (beta.31) and we have back our props in React components. 👍 💪 🙇

ArgsTable: Clean up related stories

f25ed2d

shilman requested review from ndelangen and tmeasday as code owners June 9, 2020 21:22

shilman changed the title ~~ArgsTable: Clean up related stories~~ ArgsTable: Clean up related stories (WIP) Jun 9, 2020

shilman added 2 commits June 10, 2020 05:41

Remove PropsTable

d52bdfa

Finish PropTable removal by creating temporary PropDef type

5a943f8

shilman requested a review from stijnkoopal as a code owner June 9, 2020 23:25

Fix typo

d61f7df

shilman added addon: docs maintenance User-facing maintenance tasks BREAKING CHANGE labels Jun 10, 2020

shilman changed the title ~~ArgsTable: Clean up related stories (WIP)~~ Components: Remove PropsTable, clean ArgsTable stories Jun 10, 2020

shilman removed the maintenance User-facing maintenance tasks label Jun 10, 2020

shilman added this to the 6.0 docs milestone Jun 10, 2020

tmeasday reviewed Jun 10, 2020

View reviewed changes

shilman added 3 commits June 10, 2020 10:48

ArgsTable stories: Respond to review comments

538f7c9

Fix DocsPage stories

4aa74d2

ArgsTable: revert tricky "story reuse" pattern

51e94df

shilman merged commit 6aaa9d4 into next Jun 10, 2020

ndelangen deleted the 10979-argstable-design-update branch June 10, 2020 11:34

shilman mentioned this pull request Jun 17, 2020

React: Fix react-docgen for JS files #11217

Merged

mkrds mentioned this pull request Feb 17, 2021

Unable to render custom ArgsTable #13944

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Components: Remove PropsTable, clean ArgsTable stories #11105

Components: Remove PropsTable, clean ArgsTable stories #11105

shilman commented Jun 9, 2020 •

edited

Loading

github-actions bot commented Jun 9, 2020 •

edited

Loading

shilman commented Jun 10, 2020 •

edited

Loading

tmeasday left a comment

tmeasday Jun 10, 2020

shilman Jun 10, 2020 •

edited

Loading

tmeasday Jun 10, 2020

ndelangen Jun 10, 2020

ndelangen Jun 10, 2020

Bruno-Tavares-Bose commented Jun 15, 2020 •

edited

Loading

shilman commented Jun 16, 2020

Bruno-Tavares-Bose commented Jun 16, 2020

shilman commented Jun 16, 2020

Bruno-Tavares-Bose commented Jun 16, 2020

hipstersmoothie commented Jun 16, 2020

hipstersmoothie commented Jun 16, 2020 •

edited

Loading

Bruno-Tavares-Bose commented Jun 16, 2020

hipstersmoothie commented Jun 16, 2020 •

edited

Loading

hipstersmoothie commented Jun 16, 2020

shilman commented Jun 17, 2020

hipstersmoothie commented Jun 17, 2020

shilman commented Jun 17, 2020 •

edited

Loading

hipstersmoothie commented Jun 17, 2020

Bruno-Tavares-Bose commented Jun 18, 2020

Components: Remove PropsTable, clean ArgsTable stories #11105

Components: Remove PropsTable, clean ArgsTable stories #11105

Conversation

shilman commented Jun 9, 2020 • edited Loading

What I did

How to test

github-actions bot commented Jun 9, 2020 • edited Loading

shilman commented Jun 10, 2020 • edited Loading

tmeasday left a comment

Choose a reason for hiding this comment

tmeasday Jun 10, 2020

Choose a reason for hiding this comment

shilman Jun 10, 2020 • edited Loading

Choose a reason for hiding this comment

tmeasday Jun 10, 2020

Choose a reason for hiding this comment

ndelangen Jun 10, 2020

Choose a reason for hiding this comment

ndelangen Jun 10, 2020

Choose a reason for hiding this comment

Bruno-Tavares-Bose commented Jun 15, 2020 • edited Loading

shilman commented Jun 16, 2020

Bruno-Tavares-Bose commented Jun 16, 2020

shilman commented Jun 16, 2020

Bruno-Tavares-Bose commented Jun 16, 2020

hipstersmoothie commented Jun 16, 2020

hipstersmoothie commented Jun 16, 2020 • edited Loading

Bruno-Tavares-Bose commented Jun 16, 2020

hipstersmoothie commented Jun 16, 2020 • edited Loading

hipstersmoothie commented Jun 16, 2020

shilman commented Jun 17, 2020

hipstersmoothie commented Jun 17, 2020

shilman commented Jun 17, 2020 • edited Loading

hipstersmoothie commented Jun 17, 2020

Bruno-Tavares-Bose commented Jun 18, 2020

shilman commented Jun 9, 2020 •

edited

Loading

github-actions bot commented Jun 9, 2020 •

edited

Loading

shilman commented Jun 10, 2020 •

edited

Loading

shilman Jun 10, 2020 •

edited

Loading

Bruno-Tavares-Bose commented Jun 15, 2020 •

edited

Loading

hipstersmoothie commented Jun 16, 2020 •

edited

Loading

hipstersmoothie commented Jun 16, 2020 •

edited

Loading

shilman commented Jun 17, 2020 •

edited

Loading