-
Notifications
You must be signed in to change notification settings - Fork 6
Provide stories with argTypes for primitives #3
Changes from 14 commits
a4bad83
4aa29f8
40283b1
bce4bda
6b7e54a
3e3c241
2e9bdc5
916bda8
5a9208d
fb1bbef
8c04a13
2b74c8a
4a5547c
fd6e91d
630e9c1
9872664
01987c8
47b7793
286f45c
c11ea4e
8d31cac
97fe83f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
module.exports = { | ||
"stories": [ | ||
"../src/**/*.stories.mdx", | ||
"../src/**/*.stories.@(js|jsx|ts|tsx)" | ||
], | ||
"addons": [ | ||
"@storybook/addon-links", | ||
"@storybook/addon-essentials", | ||
"@storybook/addon-interactions" | ||
], | ||
"framework": "@storybook/react" | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
export const parameters = { | ||
actions: { argTypesRegex: "^on[A-Z].*" }, | ||
controls: { | ||
matchers: { | ||
color: /(background|color)$/i, | ||
date: /Date$/, | ||
}, | ||
}, | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,30 @@ | ||
#!/usr/bin/env node --experimental-loader esbuild-node-loader | ||
|
||
import path from "path"; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note to self: need to add prettier config There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
import docgen from "react-docgen-typescript"; | ||
import fg from "fast-glob" | ||
import fs from "fs-extra" | ||
import {propsToArgTypes} from "../src/arg-types/utils"; | ||
|
||
const tsConfigPath = path.resolve("./tsconfig.json") | ||
|
||
const options = { | ||
shouldExtractLiteralValuesFromEnum: true, | ||
shouldRemoveUndefinedFromOptional: true, | ||
} | ||
|
||
// Create a parser with using your typescript config | ||
const tsConfigParser = docgen.withCustomConfig(tsConfigPath, options); | ||
|
||
// Search for components | ||
const componentFiles = fg.sync(['./src/components/*.tsx', '!./src/**/*.stories.tsx']); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Probably should be an arg to the script as a single pattern |
||
|
||
// For each component file generate argTypes based on the propTypes | ||
componentFiles.forEach(filePath => { | ||
const jsonPath = filePath.replace('.tsx', '.props.json') | ||
const res = tsConfigParser.parse(filePath) | ||
const argTypes = propsToArgTypes(res[0].props) | ||
fs.ensureFileSync(jsonPath) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think you don't need this line because its fs-extras, makes sure path exists, correct me if I am wrong There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No, it doesn't. |
||
fs.writeJsonSync(jsonPath, argTypes) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can pass options to format it with 2 spaces and make it human readable and also easy to see future changes in a diff |
||
console.log(`Done generating ${jsonPath}`) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,71 @@ | ||
export const globalArgTypes = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can be removed? |
||
accessKey: { | ||
description: "Keyboard shortcut to activate or add focus to the element.", | ||
control: "text", | ||
}, | ||
autofocus: { | ||
defaultValue: false, | ||
description: | ||
"The element should be automatically focused after the page loaded.", | ||
control: "boolean", | ||
}, | ||
disabled: { | ||
defaultValue: false, | ||
description: "Indicates whether the user can interact with the element.", | ||
control: "boolean", | ||
}, | ||
formAction: { | ||
description: | ||
"Indicates the action of the element, overriding the action defined in the <form>", | ||
control: "text", | ||
}, | ||
formEncType: { | ||
description: `If the button/input is a submit button (type="submit"), this attribute sets the encoding type to use during form submission. If this attribute is specified, it overrides the enctype attribute of the button's form owner.`, | ||
defaultValue: "application/x-www-form-urlencoded", | ||
control: "select", | ||
options: [ | ||
"application/x-www-form-urlencoded", | ||
"multipart/form-data", | ||
"text/plain", | ||
], | ||
}, | ||
formMethod: { | ||
description: `If the button/input is a submit button (type="submit"), this attribute sets the submission method to use during form submission (GET, POST, etc.). If this attribute is specified, it overrides the method attribute of the button's form owner.`, | ||
defaultValue: "get", | ||
control: "radio", | ||
options: ["post", "get", "dialog"], | ||
}, | ||
formNoValidate: { | ||
description: `If the button/input is a submit button (type="submit"), this boolean attribute specifies that the form is not to be validated when it is submitted. If this attribute is specified, it overrides the novalidate attribute of the button's form owner.`, | ||
defaultValue: true, | ||
control: "boolean", | ||
}, | ||
formTarget: { | ||
description: [ | ||
"A string reflecting a name or keyword indicating where to display the response that is received after submitting the form. If specified, this attribute overrides the target attribute of the <form> element that owns this element.", | ||
"Indicates where to display the response after submitting the form. In HTML 4, this is the name/keyword for a frame. In HTML5, it is a name/keyword for a browsing context (for example, tab, window, or iframe). The following keywords have special meanings:", | ||
"_self (default): Load into the same browsing context as the current one.", | ||
"_blank: Load into a new unnamed browsing context.", | ||
"_parent: Load into the parent browsing context of the current one. If no parent, behaves the same as _self.", | ||
"_top: Load into the top-level browsing context (i.e., the browsing context that is an ancestor of the current one and has no parent). If no parent, behaves the same as _self.", | ||
].join("\n"), | ||
defaultValue: "_self", | ||
control: "text", | ||
}, | ||
name: { | ||
description: | ||
"A string representing the name of the element when submitted with a form. If specified, it must not be the empty string.", | ||
control: "text", | ||
}, | ||
tabIndex: { | ||
defaultValue: 0, | ||
description: | ||
"Overrides the browser's default tab order and follows the one specified instead.", | ||
control: "number", | ||
}, | ||
value: { | ||
description: | ||
"Defines a default value which will be displayed in the element on page load.", | ||
control: "text", | ||
}, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,24 @@ | ||
const path = require('path') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think loader isn't used any more, right? |
||
const docgen = require("react-docgen-typescript"); | ||
|
||
module.exports = function() { | ||
const filePath = this.request.split('!').pop() || ''; | ||
const tsConfigPath = path.resolve(__dirname, "../../tsconfig.json") | ||
|
||
const options = { | ||
shouldExtractLiteralValuesFromEnum: true, | ||
// shouldExtractValuesFromUnion: true, | ||
shouldRemoveUndefinedFromOptional: true, | ||
} | ||
|
||
// Create a parser with using your typescript config | ||
const tsConfigParser = docgen.withCustomConfig(tsConfigPath, options); | ||
const res = tsConfigParser.parse(filePath) | ||
|
||
return ` | ||
if (module.hot) { | ||
module.hot.accept([]) | ||
} | ||
module.exports = ${JSON.stringify(res[0].props)} | ||
`; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,78 @@ | ||
import type {ArgTypes} from "@storybook/csf" | ||
import {PropItem} from "react-docgen-typescript"; | ||
|
||
export type FilterPredicate = (prop: PropItem) => boolean | ||
|
||
const validAttributes = (prop: PropItem) => { | ||
if (prop.parent) { | ||
return ['ButtonHTMLAttributes', 'HTMLAttributes', 'AriaAttributes'].includes(prop.parent.name) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This needs to be extended for all elements? |
||
} | ||
return true | ||
} | ||
|
||
export const propsToArgTypes = (props: Record<string, PropItem>, filter?: FilterPredicate): ArgTypes => { | ||
const filterFn = filter ?? validAttributes | ||
const entries = Object.entries(props); | ||
return entries | ||
.reduce((result, current) => { | ||
const [propName, prop] = current | ||
|
||
// Filter out props | ||
if (!filterFn(prop)) { | ||
return result | ||
} | ||
|
||
const control = mapControlForType(prop) | ||
result[propName] = {...prop, ...control} | ||
return result | ||
}, {} as ArgTypes); | ||
} | ||
|
||
const matchers = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. needs to be extended to support all controls? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's how it's done in Storybook now. I would not do it here at all TBH. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hmm, does this compromise our ability to render controls in both correctly? I guess if we do a better job at rendering than storybook its not a problem, unless we become incompatible |
||
color: new RegExp('(background|color)', 'i'), | ||
date: /Date$/ | ||
} | ||
|
||
export const mapControlForType = (propItem: PropItem): any => { | ||
const {type, name} = propItem; | ||
if (!type) { | ||
return undefined; | ||
} | ||
|
||
// args that end with background or color e.g. iconColor | ||
if (matchers.color && matchers.color.test(name)) { | ||
const controlType = propItem.type.name; | ||
|
||
if (controlType === 'string') { | ||
return { control: { type: 'color' }, defaultValue: propItem.defaultValue?.value }; | ||
} | ||
} | ||
|
||
// args that end with date e.g. purchaseDate | ||
if (matchers.date && matchers.date.test(name)) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this might be too smart, because a field with purchaseDate could actually expect a non-date string or some kind of specific format and we would be probably enforcing an ISO date or something ... unless the type in typescript is an actual date, we probably shouldn't guess like this There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree it's too "smart" and it's presentational logic that IMO should live in the designer, not in the data set. I would not even add There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, but then we would loose the interop with storybook There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not really. We could do enhance the data with There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I imagine the future where a user can provide their components with stories and those could be potentially manually written, right? So in this case we say: we always generate argTypes ourselves, no matter what user has defined in the story? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What if user doesn't have types ? :) |
||
return { control: { type: 'date' } }; | ||
} | ||
|
||
switch (type?.name) { | ||
case 'array': | ||
return {control: {type: 'object'}}; | ||
case 'boolean': | ||
case 'Booleanish': | ||
return {control: {type: 'boolean'}}; | ||
case 'string': | ||
return {control: {type: 'text'}}; | ||
case 'number': | ||
return {control: {type: 'number'}}; | ||
case 'enum': { | ||
const {value} = type; | ||
// @ts-ignore | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needs type fix There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Replaced with |
||
const values = value.map(val => val.value) | ||
return {control: {type: values?.length <= 5 ? 'radio' : 'select'}, options: values}; | ||
} | ||
case 'function': | ||
case 'symbol': | ||
return null | ||
default: | ||
return {control: {type: 'text'}}; | ||
} | ||
}; |
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import React from "react"; | ||
import type { ComponentStory, ComponentMeta } from "@storybook/react"; | ||
import { Bold as BoldPrimitive } from "./bold"; | ||
import argTypes from "./bold.props.json" | ||
|
||
export default { | ||
title: "Components/Bold", | ||
component: BoldPrimitive, | ||
argTypes: {}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. forgotten one There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed |
||
} as ComponentMeta<typeof BoldPrimitive>; | ||
|
||
const Template: ComponentStory<typeof BoldPrimitive> = (args) => ( | ||
<BoldPrimitive {...args} /> | ||
); | ||
|
||
export const Bold = Template.bind({}); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Note to self: we can rewrite these to just exporting a component that renders the primitive instead of using .args, right? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It can be done, but it will disable There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. right, forgot about it, we need them |
||
Bold.args = { | ||
children: "some bold text", | ||
}; |
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
import React from "react"; | ||
import type { ComponentStory, ComponentMeta } from "@storybook/react"; | ||
import { Box as BoxPrimitive } from "./box"; | ||
import argTypes from "./box.props.json" | ||
|
||
export default { | ||
title: "Components/Box", | ||
component: BoxPrimitive, | ||
argTypes, | ||
} as ComponentMeta<typeof BoxPrimitive>; | ||
|
||
const Template: ComponentStory<typeof BoxPrimitive> = (args) => ( | ||
<BoxPrimitive | ||
{...args} | ||
style={{ minHeight: 20, outline: "1px solid black" }} | ||
/> | ||
); | ||
|
||
export const Box = Template.bind({}); | ||
Box.args = {}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,10 @@ | ||
import React, { forwardRef, type HTMLProps } from "react"; | ||
import React, { forwardRef, type ElementRef, type ComponentProps } from "react"; | ||
|
||
export const Box = forwardRef<HTMLDivElement, HTMLProps<HTMLDivElement>>( | ||
(props, ref) => <div {...props} ref={ref} /> | ||
); | ||
const defaultTag = "div"; | ||
|
||
export const Box = forwardRef< | ||
ElementRef<typeof defaultTag>, | ||
ComponentProps<typeof defaultTag> | ||
>((props, ref) => <div {...props} ref={ref} />); | ||
|
||
Box.displayName = "Box"; |
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
import React from "react"; | ||
import type { ComponentMeta, ComponentStory } from "@storybook/react"; | ||
import { Button } from "./button"; | ||
import argTypes from "./button.props.json" | ||
|
||
export default { | ||
title: "Components/Button", | ||
component: Button, | ||
argTypes, | ||
} as ComponentMeta<typeof Button>; | ||
|
||
const Template: ComponentStory<typeof Button> = (args) => ( | ||
<Button {...args} /> | ||
); | ||
|
||
export const Example = Template.bind({}); | ||
|
||
Example.args = { | ||
children: "Test" | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,8 +1,10 @@ | ||
import React, { forwardRef, type HTMLProps } from "react"; | ||
import React, { forwardRef, type ElementRef, type ComponentProps } from "react"; | ||
|
||
const defaultTag = "button"; | ||
|
||
export const Button = forwardRef< | ||
HTMLButtonElement, | ||
HTMLProps<HTMLButtonElement> | ||
>((props, ref) => <button {...props} type="submit" ref={ref} />); | ||
ElementRef<typeof defaultTag>, | ||
ComponentProps<typeof defaultTag> | ||
>((props, ref) => <button {...props} ref={ref} />); | ||
|
||
Button.displayName = "Button"; |
Large diffs are not rendered by default.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
import React from "react"; | ||
import type { ComponentStory, ComponentMeta } from "@storybook/react"; | ||
import { Form as FormPrimitive } from "./form"; | ||
import argTypes from "./form.props.json" | ||
|
||
export default { | ||
title: "Components/Form", | ||
component: FormPrimitive, | ||
argTypes, | ||
} as ComponentMeta<typeof FormPrimitive>; | ||
|
||
const Template: ComponentStory<typeof FormPrimitive> = (args) => ( | ||
<FormPrimitive {...args} /> | ||
); | ||
|
||
export const Form = Template.bind({}); | ||
Form.args = {}; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: convention for file names is dash separated lower case
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note to self: need a linter for this
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any reason why BTW? I prefer CamelCased (like in code) especially to match component names.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I used to like camel cased too because it matches component names, but I am not using default export any more most of the time and components export more than one thing, so there is no matching, also I don't encourage putting a component per module.
Additionally it avoids case-sensitivity file system/git issues you are probably aware of