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): use native storybook/angular executor #9332

Merged
merged 1 commit into from
May 19, 2022

Conversation

mandarini
Copy link
Member

@mandarini mandarini commented Mar 15, 2022

Current Behavior

Right now, when we're invoking the Storybook target, the @nrwl/storybook:storybook runs, which is basically a wrapper around buildDevStandalone of Storybook. This may lead to inconsistencies between the native Storybook result and the Nx result. Plus, it brings the need for maintenance.

Expected Behavior

If this works out, in the future, we're hoping to be using the native Storybook executors to serve and build Storybook, instead of the nx-wrapped ones, for Angular projects.

eg.

    "storybook": {
      "executor": "@storybook/angular:start-storybook",
      ....

To try this out

Transform your existing Angular project storybook and build-storybook targets

Use the generator @nrwl/storybook:change-storybook-targets to change your existing Angular project Storybook targets to use the native executor.

Then run the storybook executor as you would normally (nx storybook <PROJECT-NAME>)

Generate Storybook Configuration for a new Angular project

The new configuration will have the new targets

Sample Repo

Check out this repository - it shows all use cases.

@nx-cloud
Copy link

nx-cloud bot commented Mar 15, 2022

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 4a1eda0. 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 12 targets

Sent with 💌 from NxCloud.

@vercel
Copy link

vercel bot commented Mar 15, 2022

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/nrwl/nx-dev/HykM5xDKbUoYfn26i7ZZqLZwAR7y
✅ Preview: https://nx-dev-git-fork-mandarini-feat-change-storybook-executor-nrwl.vercel.app

[Deployment for daefc71 canceled]

Copy link
Contributor

@Coly010 Coly010 left a comment

Choose a reason for hiding this comment

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

Code looks good, let me check it out locally and do a test or two :)

packages/devkit/src/executors/parse-target-string.ts Outdated Show resolved Hide resolved
@Coly010
Copy link
Contributor

Coly010 commented Mar 16, 2022

When building locally I had a TS error come from update-12-3-0/update-storybook migration In the angular package.

Error: packages/angular/src/migrations/update-12-3-0/update-storybook.ts:39:11 - error TS2367: This condition will always return 'true' since the types '"@storybook/react" | "@storybook/react-native" | "@storybook/html" | "@storybook/web-components" | "@storybook/vue" | "@storybook/vue3" | "@storybook/svelte"' and '"@storybook/angular"' have no overlap.

39       if (options.uiFramework !== '@storybook/angular') {

Commenting that migration out, I can build. Which let me test it out.

And it works well on a newly generated angular app! 🔥

And it also works for MFE apps where we end up changing the default builder etc.

Awesome :)

@mandarini mandarini requested a review from rarmatei March 16, 2022 17:20
@mandarini mandarini changed the title Feat/change storybook executor feat(storybook): use native storybook angular executor Mar 16, 2022
@mandarini mandarini force-pushed the feat/change-storybook-executor branch 3 times, most recently from 954c26c to d6c2ffc Compare March 16, 2022 18:25
@mandarini mandarini self-assigned this Mar 16, 2022
@mandarini mandarini force-pushed the feat/change-storybook-executor branch from d6c2ffc to e40938f Compare March 17, 2022 10:12
@mandarini mandarini force-pushed the feat/change-storybook-executor branch from e40938f to c9be5a5 Compare March 17, 2022 10:32
@mandarini mandarini force-pushed the feat/change-storybook-executor branch from c9be5a5 to f289897 Compare March 17, 2022 12:42
@mandarini mandarini force-pushed the feat/change-storybook-executor branch from f289897 to 02c6189 Compare March 18, 2022 14:46
@mandarini mandarini requested a review from juristr March 18, 2022 14:56
@mandarini mandarini force-pushed the feat/change-storybook-executor branch from 02c6189 to 00540de Compare March 28, 2022 11:32
@mandarini mandarini force-pushed the feat/change-storybook-executor branch from 4adcb71 to ecee1da Compare May 10, 2022 12:20
@mandarini mandarini marked this pull request as ready for review May 10, 2022 12:21
@mandarini mandarini requested a review from Coly010 May 10, 2022 12:21
@mandarini mandarini force-pushed the feat/change-storybook-executor branch from ecee1da to 89792f9 Compare May 10, 2022 12:40
@mandarini mandarini force-pushed the feat/change-storybook-executor branch 2 times, most recently from 0fb8877 to 6ccdcee Compare May 10, 2022 12:50
@mandarini mandarini removed the request for review from kirjai May 10, 2022 13:01
@mandarini mandarini force-pushed the feat/change-storybook-executor branch from 6ccdcee to d4c53bb Compare May 10, 2022 13:17
@Coly010
Copy link
Contributor

Coly010 commented May 13, 2022

This looks good on my side!

There's a conflict the docs map, but otherwise this looks good!

@github-actions
Copy link

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 Mar 18, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
scope: storybook Issues related to Storybook support in Nx
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants