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.

Sign up for GitHub

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

add TS support for storybook preview tsx config extension #9309

Merged
merged 27 commits into from
Dec 25, 2023
Merged

add TS support for storybook preview tsx config extension #9309

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
120702e
feat: update storybook paths
bnn1 Oct 5, 2023
d5b9929
Merge branch 'main' of github.com:bnn1/redwood into 9118/storybook-ts…
bnn1 Oct 5, 2023
26cb17f
feat: create storybook tsx templates
bnn1 Oct 5, 2023
737feba
feat: update storybook template paths
bnn1 Oct 5, 2023
2fabf18
feat: remove storybook js templates
bnn1 Oct 5, 2023
2ca7f25
Merge branch 'main' of github.com:bnn1/redwood into 9118/storybook-ts…
bnn1 Oct 17, 2023
a20e396
feat: add support for ts/tsx files
bnn1 Oct 17, 2023
3953f32
feat: use tsx storybook template
bnn1 Oct 17, 2023
a373973
feat: read tsx storybook preview config if the project is ts
bnn1 Oct 17, 2023
1e3bf75
fix: failing tests
bnn1 Oct 18, 2023
1904f2a
revert: ts support for storybook manager and config files
bnn1 Oct 18, 2023
b57d724
Merge branch 'redwoodjs:main' into 9118/storybook-tsx-support
bnn1 Oct 18, 2023
19b6d79
Merge branch 'main' of github.com:bnn1/redwood into 9118/storybook-ts…
bnn1 Oct 18, 2023
8a292c0
revert: type annotations for js storybook preview files
bnn1 Oct 18, 2023
bc74dcf
Merge branch '9118/storybook-tsx-support' of github.com:bnn1/redwood …
bnn1 Oct 18, 2023
3f713a4
Merge branch 'main' of github.com:bnn1/redwood into 9118/storybook-ts…
bnn1 Nov 6, 2023
40aabfd
feat: include 'config' directory in web tsconfig
bnn1 Nov 6, 2023
a476d18
fix: regenerate fixtures, crwa, fix comment type
bnn1 Nov 6, 2023
2c5e9d0
fix: add backward compatibility for ts users
bnn1 Nov 7, 2023
6e124df
Merge branch 'main' of github.com:bnn1/redwood into 9118/storybook-ts…
bnn1 Dec 1, 2023
36a0323
fix: pr
bnn1 Dec 2, 2023
de828f0
fix: failing tests
bnn1 Dec 2, 2023
44bdd24
Merge branch 'main' into 9118/storybook-tsx-support
dac09 Dec 5, 2023
b1926bb
Merge branch 'redwoodjs:main' into 9118/storybook-tsx-support
bnn1 Dec 24, 2023
79ef0f0
fix: pr suggestions
bnn1 Dec 24, 2023
a0f91f6
feat: jsx/tsx/ts merge tests
bnn1 Dec 24, 2023
a91cc77
Merge branch 'main' into 9118/storybook-tsx-support
dac09 Dec 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions __fixtures__/test-project/web/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
},
"include": [
"src",
"config",
"../.redwood/types/includes/all-*",
"../.redwood/types/includes/web-*",
"../types",
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands/setup/i18n/i18nHandler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -171,7 +171,7 @@ export const handler = async ({ force }) => {
skip: () => fileIncludes(rwPaths.web.storybookConfig, 'withI18n'),
task: async () =>
extendStorybookConfiguration(
path.join(__dirname, 'templates', 'storybook.preview.js.template')
path.join(__dirname, 'templates', 'storybook.preview.tsx.template')
),
},
{
Expand Down
12 changes: 6 additions & 6 deletions ...n/templates/storybook.preview.js.template → .../templates/storybook.preview.tsx.template
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,11 @@
import * as React from 'react'
import { I18nextProvider } from 'react-i18next'
import type { GlobalTypes } from '@storybook/csf'
import type { StoryFn, StoryContext } from '@storybook/react'
import i18n from 'web/src/i18n'

/** @type { import("@storybook/csf").GlobalTypes } */
export const globalTypes = {
export const globalTypes: GlobalTypes = {
locale: {
name: 'Locale',
description: 'Internationalization locale',
Expand All @@ -23,12 +25,10 @@ export const globalTypes = {
* https://github.com/storybookjs/addon-kit/blob/main/src/withGlobals.ts
* Unfortunately that will make eslint complain, so we have to disable it when
* using a hook below
*
* @param { import("@storybook/addons").StoryFn} StoryFn
* @param { import("@storybook/addons").StoryContext} context
* @returns a story wrapped in an I18nextProvider
* @param { import("@storybook/react").StoryFn} StoryFn
* @param { import("@storybook/react").StoryContext} context
*/
const withI18n = (StoryFn, context) => {
const withI18n = (StoryFn: StoryFn, context: StoryContext) => {
// eslint-disable-next-line react-hooks/rules-of-hooks
React.useEffect(() => {
i18n.changeLanguage(context.globals.locale)
Expand Down
2 changes: 1 addition & 1 deletion packages/cli/src/commands/setup/ui/libraries/chakra-ui.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ export async function handler({ force, install }) {
__dirname,
'..',
'templates',
'chakra.storybook.preview.js.template'
'chakra.storybook.preview.tsx.template'
)
),
},
Expand Down
5 changes: 3 additions & 2 deletions packages/cli/src/commands/setup/ui/libraries/mantine.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -158,14 +158,15 @@ export async function handler({ force, install, packages }) {
},
{
title: 'Configure Storybook...',
skip: () => fileIncludes(rwPaths.web.storybookConfig, 'withMantine'),
skip: () =>
fileIncludes(rwPaths.web.storybookPreviewConfig, 'withMantine'),
task: async () =>
extendStorybookConfiguration(
path.join(
__dirname,
'..',
'templates',
'mantine.storybook.preview.js.template'
'mantine.storybook.preview.tsx.template'
)
),
},
Expand Down
8 changes: 6 additions & 2 deletions ...ates/chakra.storybook.preview.js.template → ...tes/chakra.storybook.preview.tsx.template
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
import * as React from 'react'

import { ChakraProvider, extendTheme } from '@chakra-ui/react'
import * as theme from 'config/chakra.config'
import type { StoryFn } from '@storybook/react'
import theme from 'config/chakra.config'

const extendedTheme = extendTheme(theme)

const withChakra = (StoryFn) => {
/**
* @param { import("@storybook/react").StoryFn} StoryFn
*/
const withChakra = (StoryFn: StoryFn) => {
return (
<ChakraProvider theme={extendedTheme}>
<StoryFn />
Expand Down
6 changes: 5 additions & 1 deletion ...tes/mantine.storybook.preview.js.template → ...es/mantine.storybook.preview.tsx.template
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,15 @@
import * as React from 'react'

import { MantineProvider } from '@mantine/core'
import type { StoryFn } from '@storybook/react'
import theme from 'config/mantine.config'

import '@mantine/core/styles.css'

const withMantine = (StoryFn) => {
/**
* @param { import("@storybook/react").StoryFn} StoryFn
*/
const withMantine = (StoryFn: StoryFn) => {
return (
<MantineProvider theme={theme}>
<StoryFn />
Expand Down
30 changes: 30 additions & 0 deletions packages/cli/src/lib/__tests__/mergeBasics.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,36 @@ describe('the basics', () => {
{ ArrayExpression: concatUnique }
)
})
it('Merges JSX strings', () => {
const componentA = 'const ComponentA = (props) => <div>Hello</div>'
const componentB = 'const ComponentB = (props) => <div>Bye</div>'
expectTrivialConcat(componentA, componentB)
})
it('Merges TSX strings', () => {
const componentA =
'const ComponentA: MyComponent = (props) => <div>Hello</div>'
const componentB =
'const ComponentB: MyComponent = (props) => <div>Bye</div>'
expectTrivialConcat(componentA, componentB)
})
it('Merges TS strings', () => {
expectMerged(
`\
const x: string = 'x'
const list: string[] = [x]
`,
`\
const y: string = 'y'
const list: string[] = [y]
`,
`\
const x: string = 'x'
const y: string = 'y'
const list: string[] = [x, y]
`,
{ ArrayExpression: concatUnique }
)
})
})

describe('Import behavior', () => {
Expand Down
41 changes: 31 additions & 10 deletions packages/cli/src/lib/configureStorybook.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import path from 'path'
import util from 'util'

import fse from 'fs-extra'
import prettier from 'prettier'
Expand All @@ -11,24 +10,46 @@ import {
keepBoth,
keepBothStatementParents,
} from './merge/strategy'
import { isTypeScriptProject } from './project'

import { getPaths } from '.'
import { getPaths, transformTSToJS, writeFile } from '.'

/**
* Extends the Storybook configuration file with the new configuration file
* @param {string} newConfigPath - The path to the new configuration file
*/
export default async function extendStorybookConfiguration(
newConfigPath = undefined
) {
const sbPreviewConfigPath = getPaths().web.storybookPreviewConfig
const webPaths = getPaths().web
const ts = isTypeScriptProject()
const sbPreviewConfigPath =
webPaths.storybookPreviewConfig ??
`${webPaths.config}/storybook.preview.${ts ? 'tsx' : 'js'}`
const read = (path) => fse.readFileSync(path, { encoding: 'utf-8' })

if (!fse.existsSync(sbPreviewConfigPath)) {
await util.promisify(fse.cp)(
path.join(__dirname, 'templates', 'storybook.preview.js.template'),
sbPreviewConfigPath
// If the Storybook preview config file doesn't exist, create it from the template
const templateContent = read(
path.resolve(__dirname, 'templates', 'storybook.preview.tsx.template')
)
const storybookPreviewContent = ts
? templateContent
: transformTSToJS(sbPreviewConfigPath, templateContent)

await writeFile(sbPreviewConfigPath, storybookPreviewContent)
}

const storybookPreviewContent = read(sbPreviewConfigPath)

if (newConfigPath) {
const read = (path) => fse.readFileSync(path, { encoding: 'utf-8' })
const write = (path, data) => fse.writeFileSync(path, data)
const merged = merge(read(sbPreviewConfigPath), read(newConfigPath), {
// If the new config file path is provided, merge it with the Storybook preview config file
const newConfigTemplate = read(newConfigPath)
const newConfigContent = ts
? newConfigTemplate
: transformTSToJS(newConfigPath, newConfigTemplate)

const merged = merge(storybookPreviewContent, newConfigContent, {
ImportDeclaration: interleave,
ArrayExpression: concatUnique,
ObjectExpression: concatUnique,
Expand All @@ -41,6 +62,6 @@ export default async function extendStorybookConfiguration(
...(await prettier.resolveConfig(sbPreviewConfigPath)),
})

write(sbPreviewConfigPath, formatted)
writeFile(sbPreviewConfigPath, formatted, { overwriteExisting: true })
}
}
3 changes: 2 additions & 1 deletion packages/cli/src/lib/merge/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -212,7 +212,8 @@ function mergeAST(baseAST, extAST, strategy = {}) {
export function merge(base, extension, strategy) {
function parseReact(code) {
return parse(code, {
Copy link
Contributor

Choose a reason for hiding this comment

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

Mind elaborating what this change was for please? I don't really have context, so hard to figure out :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah got it. So questions here:

  • is filename necessary?
  • can we just use preset-typescript instead of both plugin and preset? I believe the preset has the plugin built-in

Copy link
Contributor Author

@bnn1 bnn1 Dec 2, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The change is necessary because preset-react won't work with TS files.
image
Without the filename I get this error on the Redwood project. You're right about @babel/preset-react - it's not needed

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey, so I think this will fail when using .js files, because preset-typescript will only parse jsx when https://babeljs.io/docs/babel-preset-typescript#istsx is forced to true. I'm not 100% sure because of how the merge is being used.

My suggestion here:

  • keep babel/preset-react
  • also include preset-typescript
  • Add a few more unit tests that cover the usecases in this PR

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 tested both ts and js projects manually, everything's working. I'll add unit tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ahhh... maybe because the filename has .tsx in it, it automatically enables JSX parsing!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you have an idea what tests to write? Merge two files and test that the merged file has contents of both files? I.e.

// file1.ts
const varA = "such redwood"
const sharedArray = [1, 2]
// file2.ts
const varB = "so framework"
const sharedArray = [3, 5]
// merged.ts
const varA = "such redwood"
const varB = "so framework"
const sharedArray = [1, 2, 3, 5]
// ...test.js
expect(mergedFile.toString()).to.include('varA = "such redwood"')
// test other content

Copy link
Contributor

Choose a reason for hiding this comment

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

I think the key thing to check would be that one of the files being merged having JSX in it.

Maybe the easiest way to test it would be to take a snippet of what you're actually merging and just have it as a test. The primary motivation here would be to prevent regression :)

presets: ['@babel/preset-react'],
filename: 'merged.tsx', // required to prevent babel error. The .tsx is relevant
presets: ['@babel/preset-typescript'],
})
}

Expand Down
16 changes: 0 additions & 16 deletions packages/cli/src/lib/templates/storybook.preview.js.template
View file
Open in desktop

This file was deleted.

18 changes: 18 additions & 0 deletions packages/cli/src/lib/templates/storybook.preview.tsx.template
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
import * as React from 'react'

import type { GlobalTypes } from '@storybook/csf'
import type { StoryFn, StoryContext } from '@storybook/react'

/** @type { import("@storybook/csf").GlobalTypes } */
export const globalTypes: GlobalTypes = {}

/**
* An example, no-op storybook decorator. Use a function like this to create decorators.
* @param { import("@storybook/react").StoryFn} StoryFn
* @param { import("@storybook/react").StoryContext} context
*/
const _exampleDecorator = (StoryFn: StoryFn, _context: StoryContext) => {
return <StoryFn />
}

export const decorators = []
1 change: 1 addition & 0 deletions packages/create-redwood-app/templates/js/web/jsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
},
"include": [
"src",
"config",
"../.redwood/types/includes/all-*",
"../.redwood/types/includes/web-*",
"../types",
Expand Down
1 change: 1 addition & 0 deletions packages/create-redwood-app/templates/ts/web/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
},
"include": [
"src",
"config",
"../.redwood/types/includes/all-*",
"../.redwood/types/includes/web-*",
"../types",
Expand Down
28 changes: 4 additions & 24 deletions packages/project-config/src/__tests__/paths.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,12 +131,7 @@ describe('paths', () => {
'config',
'storybook.config.js'
),
storybookPreviewConfig: path.join(
FIXTURE_BASEDIR,
'web',
'config',
'storybook.preview.js'
),
storybookPreviewConfig: null,
storybookManagerConfig: path.join(
FIXTURE_BASEDIR,
'web',
Expand Down Expand Up @@ -411,12 +406,7 @@ describe('paths', () => {
'config',
'storybook.config.js'
),
storybookPreviewConfig: path.join(
FIXTURE_BASEDIR,
'web',
'config',
'storybook.preview.js'
),
storybookPreviewConfig: null,
storybookManagerConfig: path.join(
FIXTURE_BASEDIR,
'web',
Expand Down Expand Up @@ -737,12 +727,7 @@ describe('paths', () => {
'config',
'storybook.config.js'
),
storybookPreviewConfig: path.join(
FIXTURE_BASEDIR,
'web',
'config',
'storybook.preview.js'
),
storybookPreviewConfig: null,
storybookManagerConfig: path.join(
FIXTURE_BASEDIR,
'web',
Expand Down Expand Up @@ -1020,12 +1005,7 @@ describe('paths', () => {
'config',
'storybook.config.js'
),
storybookPreviewConfig: path.join(
FIXTURE_BASEDIR,
'web',
'config',
'storybook.preview.js'
),
storybookPreviewConfig: null,
storybookManagerConfig: path.join(
FIXTURE_BASEDIR,
'web',
Expand Down
10 changes: 4 additions & 6 deletions packages/project-config/src/paths.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ export interface WebPaths {
entries: string | null
postcss: string
storybookConfig: string
storybookPreviewConfig: string
storybookPreviewConfig: string | null
storybookManagerConfig: string
dist: string
distServer: string
Expand Down Expand Up @@ -119,9 +119,8 @@ const PATH_WEB_DIR_GRAPHQL = 'web/src/graphql' // .js,.ts

const PATH_WEB_DIR_CONFIG_POSTCSS = 'web/config/postcss.config.js'
const PATH_WEB_DIR_CONFIG_STORYBOOK_CONFIG = 'web/config/storybook.config.js'
const PATH_WEB_DIR_CONFIG_STORYBOOK_PREVIEW = 'web/config/storybook.preview.js'
const PATH_WEB_DIR_CONFIG_STORYBOOK_PREVIEW = 'web/config/storybook.preview' // .js, .tsx
bnn1 marked this conversation as resolved.
Show resolved Hide resolved
const PATH_WEB_DIR_CONFIG_STORYBOOK_MANAGER = 'web/config/storybook.manager.js'

const PATH_WEB_DIR_DIST = 'web/dist'
const PATH_WEB_DIR_DIST_SERVER = 'web/dist/server'
const PATH_WEB_DIR_DIST_SERVER_ENTRY_SERVER = 'web/dist/server/entry.server.js'
Expand Down Expand Up @@ -229,9 +228,8 @@ export const getPaths = (BASE_DIR: string = getBaseDir()): Paths => {
BASE_DIR,
PATH_WEB_DIR_CONFIG_STORYBOOK_CONFIG
),
storybookPreviewConfig: path.join(
BASE_DIR,
PATH_WEB_DIR_CONFIG_STORYBOOK_PREVIEW
storybookPreviewConfig: resolveFile(
path.join(BASE_DIR, PATH_WEB_DIR_CONFIG_STORYBOOK_PREVIEW)
),
storybookManagerConfig: path.join(
BASE_DIR,
Expand Down
11 changes: 6 additions & 5 deletions packages/testing/config/storybook/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,11 +74,12 @@ const baseConfig = {
}
}

const userPreviewPath = fs.existsSync(
redwoodProjectPaths.web.storybookPreviewConfig
)
? redwoodProjectPaths.web.storybookPreviewConfig
: './preview.example.js'
let userPreviewPath = './preview.example.js'

if (redwoodProjectPaths.storybookPreviewConfig) {
userPreviewPath = redwoodProjectPaths.storybookPreviewConfig
}

sbConfig.resolve.alias['~__REDWOOD__USER_STORYBOOK_PREVIEW_CONFIG'] =
userPreviewPath

Expand Down
Loading