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

add TS support for storybook preview tsx config extension #9309

Merged
merged 27 commits into from
Dec 25, 2023

Conversation

bnn1
Copy link
Contributor

@bnn1 bnn1 commented Oct 17, 2023

This PR enables use of storybook preview files with .tsx extensions.

Todo:

  • tests
  • add config directory to web side tsconfig.json's include
  • add type comment annotation for js storybook preview files

fixes #9118

@bnn1 bnn1 marked this pull request as draft October 18, 2023 22:43
@bnn1 bnn1 changed the title [DRAFT] 9118/storybook tsx support add TS support for storybook preview tsx config extension Oct 18, 2023
@bnn1 bnn1 marked this pull request as ready for review November 6, 2023 11:46
@bnn1
Copy link
Contributor Author

bnn1 commented Nov 6, 2023

Hey @jtoar. I've done necessary changes. Please have a look.
upd: I need to fix one thing before merging this. Right now it will be a breaking change for ts users, they will be forced to rename their storybook.preview.js to .tsx for storybook to work properly. Will upload fix tomorrow. DONE

@bnn1 bnn1 mentioned this pull request Nov 7, 2023
@beardo01
Copy link

beardo01 commented Nov 8, 2023

This would be great @bnn1! Does your Storybook work before changing the extension? Mine doesn't seem to display the Mantine UI correctly. If yours does, could you share an example of your setup? Cheers!

@bnn1
Copy link
Contributor Author

bnn1 commented Nov 8, 2023

Hey @beardo01 👋 Yes, it does work for me. However you need to import the Mantine css file inside the storybook.preview.js file.

// storybook.preview.js
import '@mantine/core/styles.css'
// rest of the file

The complete file would look like this:

import * as React from 'react'

import { MantineProvider } from '@mantine/core'
import theme from 'config/mantine.config'
import '@mantine/core/styles.css'

/** @type { import("@storybook/csf").GlobalTypes } */
export const globalTypes = {}
/**
 * An example, no-op storybook decorator. Use a function like this to create decorators.
 * @param { import("@storybook/addons").StoryFn} StoryFn
 * @param { import("@storybook/addons").StoryContext} context
 * @returns StoryFn, unmodified.
 */
const _exampleDecorator = (StoryFn, _context) => {
  return <StoryFn />
}
const withMantine = (StoryFn) => {
  return (
    <MantineProvider theme={theme}>
      <StoryFn />
    </MantineProvider>
  )
}
export const decorators = [withMantine]

This is being addressed in another PR (#9388)

@beardo01
Copy link

beardo01 commented Nov 9, 2023

Hey @beardo01 👋 Yes, it does work for me. However you need to import the Mantine css file inside the storybook.preview.js file.

Ahhh, of course you do. This fixed it for me too. Regardless, looking forward to these PRs being merged. Thanks for your hard work. If you're on the Discord I might shoot you a message to ask about your use of Mantine Forms over the built in Redwood Forms. Let me know your username if you're happy to chat that through.

@bnn1
Copy link
Contributor Author

bnn1 commented Nov 9, 2023

@beardo01 I'm on discord, my username is barbarys. You can also find me in this channel https://discord.com/channels/679514959968993311/1141095558669410456 the latest message is from me (Barys) 👋

@dac09 dac09 self-assigned this Nov 27, 2023
@dac09 dac09 self-requested a review November 27, 2023 07:39
@bnn1
Copy link
Contributor Author

bnn1 commented Dec 2, 2023

@dac09 all done. please have a look

@@ -212,7 +212,8 @@ function mergeAST(baseAST, extAST, strategy = {}) {
export function merge(base, extension, strategy) {
function parseReact(code) {
return parse(code, {
presets: ['@babel/preset-react'],
filename: 'merged.tsx',
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
filename: 'merged.tsx',
filename: 'merged.tsx', // required to prevent babel error. The .tsx is relevant

: './preview.example.js'
let userPreviewPath = './preview.example.js'

if (fs.existsSync(redwoodProjectPaths.storybookPreviewConfig)) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I believe resolveFile will handle this. If the file doesn't exist, it the value will be null

Suggested change
if (fs.existsSync(redwoodProjectPaths.storybookPreviewConfig)) {
if (redwoodProjectPaths.storybookPreviewConfig) {

@dac09
Copy link
Collaborator

dac09 commented Dec 5, 2023

I've tested both ts and js projects manually, everything's working. I'll add unit tests.

Thanks @bnn1, appreciate alll the effort into this one! Final few comments, will merge once the unit tests are in :)

@Tobbe
Copy link
Member

Tobbe commented Dec 17, 2023

@bnn1 I'm going through our open PRs and found this one. If I'm reading the comments correctly we're still missing some unit tests here. Are you still interested in writing those, so we can get this merged?

No pressure! Just let us know what you want to do 🙂

@bnn1
Copy link
Contributor Author

bnn1 commented Dec 18, 2023

Hey @Tobbe. Yess I'm interested in finishing this pr. I'll do that as soon as I can. I hope we are not time restricted there?

@Tobbe
Copy link
Member

Tobbe commented Dec 18, 2023

@bnn1 Thanks for your reply. It would be great if we could get this done before the end of the year. Is that something you could commit to, you think?

@bnn1
Copy link
Contributor Author

bnn1 commented Dec 24, 2023

@Tobbe I started working on the unit tests and it got me thinking: we are not actually merging files, we are merging file contents - strings, so file extension should not be relevant for the merge function to work as we are passing it strings and not file references. Did I misunderstand @dac09?

@Tobbe
Copy link
Member

Tobbe commented Dec 24, 2023

@bnn1 Thanks for getting started on the tests! Don't be afraid to push a WIP-commit to the PR if you get stuck or need any other kind of help.

This is 100% just a guess, but maybe the file name is needed to let babel know what "mode" to run in. What presets to use etc. If the filename ends with .tsx or .jsx it might know to handle the <MyComponent> syntax for example.

@bnn1
Copy link
Contributor Author

bnn1 commented Dec 24, 2023

@Tobbe I don't understand what tests you need 😅. As I understand, the idea was to test that merge works for .jsx and .tsx files but merge function isn't dependent on file extension. @dac09 could you please clarify what you meant when we talked about tests?

Copy link
Member

Tobbe commented Dec 24, 2023

For those tests a few different ones that call merge with one or two strings with JSX should be all we need I think

@bnn1
Copy link
Contributor Author

bnn1 commented Dec 24, 2023

Copy link
Collaborator

@dac09 dac09 left a comment

Choose a reason for hiding this comment

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

Thanks so much @bnn1 :)

@dac09 dac09 enabled auto-merge (squash) December 25, 2023 05:52
@dac09 dac09 merged commit ffb0a99 into redwoodjs:main Dec 25, 2023
31 of 32 checks passed
dac09 added a commit to dac09/redwood that referenced this pull request Dec 27, 2023
…redwood into fix/enhance-error-apollo-suspense

* 'fix/enhance-error-apollo-suspense' of github.com:dac09/redwood: (92 commits)
  chore(deps): update dependency @types/yargs to v17.0.32 (redwoodjs#9759)
  Make it easier to find useMatch docs (redwoodjs#9756)
  chore(unit tests): Use side-effect import to fix TS errors (redwoodjs#9754)
  fix(context): Refactor context (redwoodjs#9371)
  docs: Replaced deprecated <Set private> with PrivateSet within router.md (redwoodjs#9749)
  add TS support for storybook preview tsx config extension (redwoodjs#9309)
  fix(studio): Fix windows path issues (redwoodjs#9752)
  redwoodjs#9620: Update studio to support variable components (Mailer) (redwoodjs#9639)
  chore(tasks): Add comparison view to nmHoisting visualisation (redwoodjs#9751)
  chore(cli): make fs modules used in the CLI consistent (redwoodjs#9746)
  chore(tooling): Make sure console boxen print on a new line
  chore(CI): fix publish release candidate
  feat(CLI): add check node version middleware, rm `.nvmrc`, yarn engines (redwoodjs#9728)
  docs: added some clarification on serverless functions getting executed in a non-serverless environment (redwoodjs#9742)
  Fix sshExec() errors not displaying (redwoodjs#9743)
  chore(tooling): Add missing word in release tooling output
  Update Metadata docs (redwoodjs#9744)
  chore(CI): update test project fixture and CRWA for deploy target CI repo (redwoodjs#9730)
  chore(tooling): add script for getting nested dependency data (redwoodjs#9734)
  Trusted Documents docs: Proofreading corrections (redwoodjs#9737)
  ...
dac09 added a commit to dac09/redwood that referenced this pull request Dec 27, 2023
…ath-aliases

* 'main' of github.com:redwoodjs/redwood: (92 commits)
  chore(deps): update dependency @types/yargs to v17.0.32 (redwoodjs#9759)
  Make it easier to find useMatch docs (redwoodjs#9756)
  chore(unit tests): Use side-effect import to fix TS errors (redwoodjs#9754)
  fix(context): Refactor context (redwoodjs#9371)
  docs: Replaced deprecated <Set private> with PrivateSet within router.md (redwoodjs#9749)
  add TS support for storybook preview tsx config extension (redwoodjs#9309)
  fix(studio): Fix windows path issues (redwoodjs#9752)
  redwoodjs#9620: Update studio to support variable components (Mailer) (redwoodjs#9639)
  chore(tasks): Add comparison view to nmHoisting visualisation (redwoodjs#9751)
  chore(cli): make fs modules used in the CLI consistent (redwoodjs#9746)
  chore(tooling): Make sure console boxen print on a new line
  chore(CI): fix publish release candidate
  feat(CLI): add check node version middleware, rm `.nvmrc`, yarn engines (redwoodjs#9728)
  docs: added some clarification on serverless functions getting executed in a non-serverless environment (redwoodjs#9742)
  Fix sshExec() errors not displaying (redwoodjs#9743)
  chore(tooling): Add missing word in release tooling output
  Update Metadata docs (redwoodjs#9744)
  chore(CI): update test project fixture and CRWA for deploy target CI repo (redwoodjs#9730)
  chore(tooling): add script for getting nested dependency data (redwoodjs#9734)
  Trusted Documents docs: Proofreading corrections (redwoodjs#9737)
  ...
dac09 added a commit to dac09/redwood that referenced this pull request Dec 28, 2023
…p-prebuild

* 'main' of github.com:redwoodjs/redwood: (1608 commits)
  Docker: Update to work with corepack and yarn v4 (redwoodjs#9764)
  [RFC]: useRoutePaths (redwoodjs#9755)
  Adds a note about the two commands you will use with your schema to the top of the schema file (redwoodjs#8589)
  docs: Supertokens.md: Fix typo (redwoodjs#9765)
  Fix supertokens docs & integration issues (redwoodjs#9757)
  fix(apollo): Enhance error differently for Suspense Cells (redwoodjs#9640)
  SSR smoke-test: Use <Metadata /> (redwoodjs#9763)
  chore(deps): update dependency @types/qs to v6.9.11 (redwoodjs#9761)
  chore(ci): Better error handling in detectChanges.mjs (redwoodjs#9762)
  fix(path-alias): Fix aliasing of paths using ts/jsconfig (redwoodjs#9574)
  chore(deps): update dependency @types/yargs to v17.0.32 (redwoodjs#9759)
  Make it easier to find useMatch docs (redwoodjs#9756)
  chore(unit tests): Use side-effect import to fix TS errors (redwoodjs#9754)
  fix(context): Refactor context (redwoodjs#9371)
  docs: Replaced deprecated <Set private> with PrivateSet within router.md (redwoodjs#9749)
  add TS support for storybook preview tsx config extension (redwoodjs#9309)
  fix(studio): Fix windows path issues (redwoodjs#9752)
  chore(tasks): Add comparison view to nmHoisting visualisation (redwoodjs#9751)
  chore(cli): make fs modules used in the CLI consistent (redwoodjs#9746)
  ...
Tobbe pushed a commit that referenced this pull request Jan 1, 2024
@jtoar jtoar modified the milestones: next-release, v7.0.0 Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:feature This PR introduces a new feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug?]: redwood doesn't recognize storybook config files with ts, tsx extensions.
5 participants