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

feat(storybook): move tsconfig one level up #17749

Merged
merged 1 commit into from
Jun 26, 2023

Conversation

mandarini
Copy link
Member

@mandarini mandarini commented Jun 23, 2023

See this PR for context: #17745

This solves the vite paths issue.

vite-tsconfig-paths plugin will not run resolversByDir for the .storybook dir, but rather for the project's root. So it does not take into account the paths included in the .storybook/tsconfig.file. Then, it looks one level up to find resolvers.
The vite-tsconfig-paths is setting up the resolvers to match the tsconfig.json settings under the .storybook directory, and different ones for the tsconfig.*.json files it’s finding in the project root. Since we're excluding .stories.* files in the project's root tsconfig, then it's failing.

Now, if we move the .storybook/tsconfig.json file to be tsconfig.storybook.json file in the project's root, then the vite-tsconfig-paths plugin will be able to resolve the paths, then, since it will be at the root of the project.

Is it ok to remove tsconfig.json from the .storybook dir?

Yes, since only @storybook/angular is using .storybook/tsconfig.json, the other frameworks ignore it. And @storybook/angular resolves the paths correctly, so we'll keep it as is for Angular projects

Why do we need a tsconfig file for Storybook anyway?

Because we want to exclude .stories.* files from the ts compilation if we're building the actual project. And our builders import specifically the tsconfig.app|lib.json file. But of course, we want to include the .stories.* files for Storybook builder and server. So, we need this extra file to include the stories that the tsconfig.app|lib.json file exludes.

TODO (done ✅ ):

  • Write migrations
  • Test it locally

Tested the following:

  1. React + Vite: app, lib, app that imports lib which imports lib, which imports lib. Story which imports lib which imports lib. Build + Build-Storybook
  2. React + Webpack: app, lib, app that imports lib which imports lib, which imports lib. Story which imports lib which imports lib. Build + Build-Storybook
  3. Angular: app, lib, all's unchanged
  4. Tested migration

Related Issue(s)

Fixes #17257 #17156

@mandarini mandarini requested a review from a team as a code owner June 23, 2023 13:27
@mandarini mandarini requested a review from jaysoo June 23, 2023 13:27
@vercel
Copy link

vercel bot commented Jun 23, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nx-dev ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 26, 2023 0:49am

@mandarini mandarini requested a review from Coly010 June 23, 2023 13:27
@mandarini mandarini self-assigned this Jun 23, 2023
@mandarini mandarini linked an issue Jun 23, 2023 that may be closed by this pull request
4 tasks
@mandarini mandarini marked this pull request as draft June 23, 2023 13:28
@nx-cloud
Copy link

nx-cloud bot commented Jun 23, 2023

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 43cef5f. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

@mandarini mandarini marked this pull request as ready for review June 23, 2023 14:37
@mandarini mandarini requested a review from a team as a code owner June 23, 2023 14:37
@mandarini mandarini requested a review from Coly010 June 23, 2023 14:39
@mandarini mandarini force-pushed the feat/storybook-tsconfig branch from 2727902 to 284245c Compare June 23, 2023 14:45
@mandarini
Copy link
Member Author

mandarini commented Jun 23, 2023

Local testing works - like create workspace, add storybook, build and run storybook.

Needs more testing though. Do not merge yet

@github-actions
Copy link

github-actions bot commented Jul 2, 2023

This pull request has already been merged/closed. If you experience issues related to these changes, please open a new issue referencing this pull request.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 2, 2023
@mandarini mandarini deleted the feat/storybook-tsconfig branch August 22, 2023 12:32
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
3 participants