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

Angular: Add multi-project setup for ng workspaces #20559

Merged
merged 18 commits into from
Jan 18, 2023

Conversation

valentinpalkovic
Copy link
Contributor

@valentinpalkovic valentinpalkovic commented Jan 10, 2023

Resolves #20213
Resolves #14026
Resolves #19289

What I did

Remove NX dependency

Previously, we tried to read the NX configuration during initialization to figure out which tsconfig we have to adjust for Storybook to work with Angular. We remove this kind of check. Instead, as soon as we recognize nx being part of the project, we exit the initialization and point to the excellent @nrwl/storybook plugin instead.

Removed support for Angular < 14

We do not support Angular below v14 anymore (18b7618)

Remove the legacy way of using Storybook for Angular

In the past, it was possible to start Storybook independently of any Angular builder. Running yarn storybook did the job. Now Storybook is bound to an Angular project, which is set up in the angular.json file. During initialization, new entries are added to a particular project's architect section to set up Storybook. You have to run ng run <project>:storybook and ng run <project>:build-storybook now instead. (4b68d75)

Multi-project setup in Angular workspace

Before, one Storybook was set up for the entire Angular workspace. Since Storybook is now bound to a particular project, this is not the case anymore. If you have set up multiple applications/libraries within an Angular workspace, you will be asked for which particular project you would like to set up Storybook. (542c535)

Optional Compodoc generation

Previously, compodoc was set up automatically for you. You'll be asked if you'd like to set it up for a particular project.

Fixes some Typescript-related issues related to Template Stories

The stricter set of Typescript rules was causing issues with the Template Stories, which are copied/linked over to a sandbox (4f4b4e4)

Removed legacy renderer

The parameters.angularLegacyRendering option is removed. You cannot use the old legacy renderer anymore.

Flowchart

F3A2D8DC-A76C-4B9D-A7DD-91869632F034_1_201_a

How to test

  1. Create an angular sandbox (yarn task --task sandbox --template angular-cli/default-ts --no-link) (Script sometimes stops at Publishing packages. Rerun the command and start from Run the internal npm server (run-registry).
  2. cd ./sandbox/angular-cli-default-ts
  3. yarn storybook -> Check whether Storybook runs smoothly. Check whether compodoc documentation is available in the addon section. Button inputs for example has some descriptions.
  4. npx ng generate application new-application -> Creates a new application in the angular workspace
  5. <path-to-storybook>/code/lib/cli/bin/index.js init -> Initialize Storybook for new-application. Answer compodoc prompt with N.
  6. Go to angular.json and check whether projects.angular-latest.architect.storybook is configured with compodoc args and projects.new-application.architect.storybook is configured without compodoc args.
  7. Go to ./projects/new-application/.storybook/main.js and replace the content with:
const mainRoot = require('../../../.storybook/main.js');
module.exports = {
  ...mainRoot,
  previewAnnotations: [],
  stories: ["../src/**/*.mdx", "../src/**/*.stories.@(js|jsx|ts|tsx)"]
};

You must do this for testing because the sub-project does not have defined template stories. This adjustment, therefore, is only necessary due to how we set up sandboxes.

  1. npx ng run new-application:storybook
  2. yarn build-storybook -> Builds root project. Outputs Storybook dist to storybook-static
  3. npx ng run new-application:build-storybook -> Builds new-application project. Outputs Storybook dist to dist/storybook/new-application

@valentinpalkovic valentinpalkovic added feature request angular ci:daily Run the CI jobs that normally run in the daily job. labels Jan 10, 2023
@valentinpalkovic valentinpalkovic force-pushed the valentin/angular15-maintenance branch 2 times, most recently from a10a7a5 to 5c7ae17 Compare January 10, 2023 13:24
@valentinpalkovic valentinpalkovic self-assigned this Jan 10, 2023
@valentinpalkovic valentinpalkovic changed the title WIP Support multi-project setup in Angular workspaces Jan 10, 2023
@valentinpalkovic valentinpalkovic force-pushed the valentin/angular15-maintenance branch 2 times, most recently from f87fc79 to d584960 Compare January 11, 2023 11:09
@valentinpalkovic valentinpalkovic marked this pull request as ready for review January 11, 2023 15:49
@shilman shilman changed the title Support multi-project setup in Angular workspaces Angular: Support multi-project setup in ng workspaces Jan 11, 2023
@mandarini
Copy link
Contributor

That's great!!! :D Thank you very much @valentinpalkovic for pointing to the Nx plugin! I'm sure it will help with setting up Storybook for Nx. I also like the new Compodoc docs and the opt-in. I need to add this to the @nrwl/storybook generator, too. I went through the PR, mainly the parts that are either concerned with Nx or with generators. Looks good to me! :)

Also, how great is this flowchart.

Copy link
Contributor

@ThibaudAV ThibaudAV left a comment

Choose a reason for hiding this comment

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

I read quickly I don't know the context of each change 😁
But on the whole I think it's great.
And the choice for nx is good

Nice to see people working on this part 😊

@Marklb
Copy link
Member

Marklb commented Jan 12, 2023

Now that the Builder is required, should we still leave a small check to for invalid configuration. In my PR to fix tests, I also removed the deprecated so I could drop the deprecated code's tests. I added a check for the angularBrowserTarget property and logged an error message that directs them to the docs for migrating to the builder. Maybe the code just throwing an error when it happens to fail is enough, but maybe this small check could make it more obvious to people that haven't migrated.
The change I am referring to in my PR is: https://github.com/Marklb/storybook/blob/marklb/update-angular-tests/code/frameworks/angular/src/server/framework-preset-angular-cli.ts#L36

@valentinpalkovic valentinpalkovic force-pushed the valentin/angular15-maintenance branch 6 times, most recently from 7e6698a to 40d1784 Compare January 13, 2023 21:38
@valentinpalkovic valentinpalkovic force-pushed the valentin/angular15-maintenance branch 2 times, most recently from 7841693 to d015495 Compare January 16, 2023 07:58
@valentinpalkovic valentinpalkovic removed the ci:daily Run the CI jobs that normally run in the daily job. label Jan 17, 2023
Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Amazing work @valentinpalkovic ❤️

A few questions from my testing. In your test instructions:

const mainRoot = require('../../../.storybook/main.js');
module.exports = {
  ...mainRoot,
  previewAnnotations: [],
  stories: ["../src/**/*.mdx", "../src/**/*.stories.@(js|jsx|ts|tsx)"]
};

What's the purpose of previewAnnotations? Is that a typo?

Also, the sandbox generated the following in angular.json:

        "storybook": {
          "builder": "@storybook/angular:start-storybook",
          "options": {
            "configDir": ".storybook",
            "browserTarget": "angular-latest:build",
            "compodoc": false,
            "compodocArgs": [
              "-e",
              "json",
              "-d",
              "."
            ],
            "port": 6006
          }
        },

But it seems like compodoc is still running/working. Any idea what's going on there?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Done
5 participants