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

Use string literal names rule #117

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -122,6 +122,7 @@ This plugin does not support MDX files.
| [`storybook/story-exports`](./docs/rules/story-exports.md) | A story file must contain at least one story export | | <ul><li>recommended</li><li>csf</li></ul> |
| [`storybook/use-storybook-expect`](./docs/rules/use-storybook-expect.md) | Use expect from `@storybook/jest` | 🔧 | <ul><li>addon-interactions</li><li>recommended</li></ul> |
| [`storybook/use-storybook-testing-library`](./docs/rules/use-storybook-testing-library.md) | Do not use testing-library directly on stories | 🔧 | <ul><li>addon-interactions</li><li>recommended</li></ul> |
| [`storybook/use-string-literal-names`](./docs/rules/use-string-literal-names.md) | Use string literals to override a story name | | <ul><li>recommended</li></ul> |

<!-- RULES-LIST:END -->

Expand Down
62 changes: 62 additions & 0 deletions docs/rules/use-string-literal-names.md
@@ -0,0 +1,62 @@
# use-string-literal-names

<!-- RULE-CATEGORIES:START -->

**Included in these configurations**: <ul><li>recommended</li></ul>

<!-- RULE-CATEGORIES:END -->

## Rule Details

When indexing stories extracted from CSF files, Storybook automatically titles them [based on the named export](https://storybook.js.org/docs/react/api/csf#named-story-exports). Story names can be overridden by setting the `name` property:

```js
export const Simple = {
decorators: [...],
parameters: {...},
// Displays "So Simple" instead of "Simple" in Storybook's sidebar
name: 'So simple!',
}
```

One can be tempted to programmatically assign story names using code such as template literals, variable references, spread objects, function invocations, etc. However, because of limitations to static analysis, which Storybook relies on, Storybook only picks `name` properties when they are string literals: it cannot evaluate code.

Examples of **incorrect** code for this rule:

```js
export const A = { name: '1994' + 'definitely not my credit card PIN' }
export const A = { name: `N` }
export const A = { name: String(1994) }

const name = 'N'
export const A = { name }

const A = { name: `N` }
export { A }

const A = { name: String(1994) }
export { A }

const name = 'N'
const A = { name }
export { A }
```

Examples of **correct** code for this rule:

```js
export const A = { name: 'N' }
export const A = { name: 'N' }

const A = { name: 'N' }
export { A }

const A = { name: 'N' }
export { A }

const A = { name } // Not a Story (not exported)
```

## Further Reading

More discussion on issue [#111](https://github.com/storybookjs/eslint-plugin-storybook/issues/111)
1 change: 1 addition & 0 deletions lib/configs/recommended.ts
Expand Up @@ -19,6 +19,7 @@ export = {
'storybook/story-exports': 'error',
'storybook/use-storybook-expect': 'error',
'storybook/use-storybook-testing-library': 'error',
'storybook/use-string-literal-names': 'error',
},
},
{
Expand Down
43 changes: 43 additions & 0 deletions lib/rules/use-string-literal-names.ts
@@ -0,0 +1,43 @@
/**
* @fileoverview Use string literals to override a story name
* @author Charles Gruenais
*/

import { createStorybookRule } from '../utils/create-storybook-rule'
import { CategoryId } from '../utils/constants'
import { isLiteral } from '../utils/ast'
import { extractStories } from '../utils/stories'

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------

const messageId = 'useStringLiteralName' as const

export = createStorybookRule({
name: 'use-string-literal-names',
defaultOptions: [],
meta: {
type: 'problem',
docs: {
description: 'Use string literals to override a story name',
categories: [CategoryId.RECOMMENDED],
recommended: 'error',
},
messages: {
[messageId]: 'Story names can only be overridden by string literals',
},
schema: [],
},

create(context) {
return extractStories(context, (output) => {
const properties = output.getProperties(['name', 'storyName'])
properties.forEach(({ valueNode: node }) => {
if (!isLiteral(node)) {
context.report({ node, messageId })
}
})
})
},
})
23 changes: 23 additions & 0 deletions lib/types/ast.ts
@@ -0,0 +1,23 @@
import type { TSESTree } from '@typescript-eslint/utils'

export type TypedNode<NodeType> = TSESTree.Node & { type: NodeType }

export type SpreadProperty = TSESTree.SpreadElement & {
argument: TSESTree.Identifier
parent: { parent: NamedVariableDeclarator }
}

export type NamedProperty = TSESTree.Property & { key: TSESTree.Identifier }

export type NamedExportSpecifier = TSESTree.ExportSpecifier & { local: TSESTree.Identifier }

export type NamedVariableDeclarator = TSESTree.VariableDeclarator & { id: TSESTree.Identifier }

export type NamedVariableDeclaration = NamedExportSpecifier | NamedVariableDeclarator

export type NamedAssignment = TSESTree.AssignmentExpression & {
left: TSESTree.MemberExpression & {
property: TSESTree.Identifier
object: TSESTree.Identifier
}
}
2 changes: 2 additions & 0 deletions lib/types/index.ts
@@ -1,6 +1,8 @@
import { TSESLint } from '@typescript-eslint/utils'
import { CategoryId } from '../utils/constants'

export type RuleContext = Readonly<TSESLint.RuleContext<string, unknown[]>>

export type RuleModule = TSESLint.RuleModule<'', []> & {
meta: { hasSuggestions?: boolean; docs: { recommendedConfig?: 'error' | 'warn' } }
}
Expand Down
45 changes: 45 additions & 0 deletions lib/types/stories.ts
@@ -0,0 +1,45 @@
import { TSESTree } from '@typescript-eslint/utils'
import { NamedVariableDeclaration } from './ast'

type StoryDeclaration = NamedVariableDeclaration | TSESTree.Expression

export type StoryPropertyDeclaration = {
storyName: string
nameNode: TSESTree.Node
valueNode: TSESTree.Node
}

export type SpreadsMap<SpreadName = string, StoryName = string> = Map<SpreadName, StoryName[]>

export type StoriesMap<StoryExportName = string> = Map<StoryExportName, StoryDeclaration>

export type PropertiesMap<PropertyKey = string> = Map<PropertyKey, StoryPropertyDeclaration[]>

export type StoryExports<StoryExportName = string> = Map<
StoryExportName,
NamedVariableDeclaration | undefined
>

export type PropertyDefinition = {
parentName: string
propertyName: string
propertyNameNode: TSESTree.Node
propertyValueNode: TSESTree.Node
}

export type StoryPropertyCandidates<StoryPropertyKey = string> = Map<
StoryPropertyKey,
PropertyDefinition
>

export type StoryDeclarationCandidates<DeclarationName = string> = Map<
DeclarationName,
{
declarationValue: TSESTree.Expression | null
}
>

export type SpreadContentCandidates<VariableName = string, ParentName = string> = Map<
VariableName,
ParentName[]
>
134 changes: 134 additions & 0 deletions lib/utils/StoryIndexer.ts
@@ -0,0 +1,134 @@
import {
PropertiesMap,
PropertyDefinition,
SpreadContentCandidates,
SpreadsMap,
StoriesMap,
StoryDeclarationCandidates,
StoryExports,
StoryPropertyCandidates,
StoryPropertyDeclaration,
} from '../types/stories'

export class StoryIndexer {
/** List of all Stories (indexed by their exported name). */
stories: StoriesMap = new Map()

/** List of all Stories properties (indexed by name). */
properties: PropertiesMap = new Map()

/**
* List of all spreads used on Stories (indexed by their name).
* Each spread item resolves to the exported name of the Story they are
* attached to. This is used internally and has no reason to be public.
*/
private spreads: SpreadsMap = new Map()

constructor(
propertyCandidates: StoryPropertyCandidates,
declarationCandidates: StoryDeclarationCandidates,
spreadCandidates: SpreadContentCandidates,
storyExports: StoryExports
) {
this.registerStories(storyExports, declarationCandidates)
this.registerSpreads(spreadCandidates)
this.registerProperties(propertyCandidates)
}

/** Output list of stories. May be filtered by one or several names. */
public getStories(q?: string | string[]) {
const stories = [...this.stories.entries()]
const outputStories = (list: typeof stories) =>
list.map(([, storyDeclaration]) => storyDeclaration)

if (!q) return outputStories(stories)

const search = Array.isArray(q) ? q : [q]
return outputStories(
stories.filter(([storyName]) => {
return search.includes(storyName)
})
)
}

/**
* Output list of properties. May be filtered by one or several names.
* Each output property is an object containing the following:
* - `storyName`: Name of the Story the property is attached to.
* - `valueNode`: Node of property's value.
* - `nameNode`: Node of property's name.
*/
public getProperties(q?: string | string[]) {
const properties = [...this.properties.entries()]
const search = Array.isArray(q) ? q : [q]
return properties.reduce((pickedProperties, [propertyName, propertyInstances]) => {
propertyInstances.forEach((propertyInstance) => {
if (!q || search.includes(propertyName)) {
pickedProperties.push(propertyInstance)
}
})
return pickedProperties
}, [] as StoryPropertyDeclaration[])
}

/** Registers stories and map them to their declaration. */
private registerStories(
storyExports: StoryExports,
declarationCandidates: StoryDeclarationCandidates
) {
const exports = [...storyExports.entries()]
exports.forEach(([name, declaration]) => {
if (declaration) {
this.stories.set(name, declaration)
} else {
const declaration = declarationCandidates.get(name)?.declarationValue
if (declaration) this.stories.set(name, declaration)
}
})
}

/** Registers spread elements used on Stories. */
private registerSpreads(spreadCandidates: SpreadContentCandidates) {
const possibleSpreads = [...spreadCandidates.entries()]
possibleSpreads.forEach(([spreadName, parentNames]) => {
parentNames.forEach((parentName) => {
if (this.stories.has(parentName)) {
this.spreads.set(spreadName, parentNames)
}
})
})
}

/** Registers story properties. */
private registerProperties(propertyCandidates: StoryPropertyCandidates) {
const possibleProperties = [...propertyCandidates.values()]

possibleProperties.forEach((property) => {
const { parentName } = property

// This is indeed a Story property.
if (this.stories.has(parentName)) {
this.addProperty(property)
}

// Property's parent object is spread within a Story declaration.
if (this.spreads.has(parentName)) {
const [storyName] = this.spreads.get(parentName) as [string]
this.addProperty({ ...property, parentName: storyName })
}
})
}

/** Adds property to list of properties. */
private addProperty({
parentName,
propertyName: name,
propertyNameNode: nameNode,
propertyValueNode: valueNode,
}: PropertyDefinition) {
this.properties.set(name, [
...(this.properties.get(name) ?? []),
{ storyName: parentName, nameNode, valueNode },
])
}
}