nrwl / nx

Smart Monorepos · Fast CI
https://nx.dev
MIT License
23.48k stars 2.34k forks source link

Don't add dependencies of Storybook to publishable library #18175

Closed lukket closed 1 year ago

lukket commented 1 year ago

Current Behavior

After adding Storybook with Compodoc support to a publishable library, the generated package.json contains peer dependencies from Storybook's preview.js.

{
  "peerDependencies": {
    "@angular/common": "^16.1.0",
    "@angular/core": "^16.1.0",
    "@storybook/addon-docs": "7.1.0"
  }
}

Expected Behavior

The generated package.json of a publishable library shouldn't contain any dependencies from Storybook.

GitHub Repo

lukket/nrwl-nx-18175

Steps to Reproduce

  1. Create new workspace (integrated repository):npx create-nx-workspace@latest myorg --preset=ts
  2. Install Angular plugin:npm install -D @nx/angular
  3. Create package foo: nx generate @nx/angular:library --publishable --import-path=@my-org/foo foo
  4. Enable Storybook for package foo: nx generate @nx/angular:storybook-configuration foo
  5. Build package foo: nx build foo
  6. The generated package.json contains dependencies from the auto-generated Angular module.
    {
      "peerDependencies": {
        "@angular/common": "^16.1.0",
        "@angular/core": "^16.1.0"
      },  
      "dependencies": {
        "tslib": "^2.3.0"
      }
    }
  7. Follow the steps in the official Nx guide "How to enable Compodoc for Storybook" until Storybook's preview.js gets modified.
  8. Build package foo: nx build foo
  9. The generated package.json now contains dependencies from Storybook's preview.js.
    {
      "peerDependencies": {
        "@angular/common": "^16.1.0",
        "@angular/core": "^16.1.0",
        "@storybook/addon-docs": "7.1.0"
      }, 
    }

Nx Report

Node   : 16.20.1
   OS     : darwin-arm64
   npm    : 8.19.4

   nx (global)        : 16.5.2
   nx                 : 16.5.3
   @nx/js             : 16.5.3
   @nx/jest           : 16.5.3
   @nx/linter         : 16.5.3
   @nx/workspace      : 16.5.3
   @nx/angular        : 16.5.3
   @nx/cypress        : 16.5.3
   @nx/devkit         : 16.5.3
   @nx/eslint-plugin  : 16.5.3
   @nx/storybook      : 16.5.3
   @nrwl/tao          : 16.5.3
   @nx/web            : 16.5.3
   @nx/webpack        : 16.5.3
   typescript         : 5.1.6
   ---------------------------------------
   Community plugins:
   @compodoc/compodoc : 1.1.21
   @storybook/angular : 7.1.0
   ---------------------------------------
   Local workspace plugins:
         @my-org/foo

Failure Logs

No response

Operating System

Additional Information

There are related issues #15314 and #15039 and PR #13966, but the default behavior still seems broken. It's also not clear how the mentioned PR helps to work around this issue.

lukket commented 1 year ago

@mandarini could you explain how PR #13966 can help to work around this issue, as mentioned in the related issues #15314 and #15039?

meeroslav commented 1 year ago

Hey @lukket,

Thank you for reporting this. By default, we collect dependencies from all the project's source files that are part of the production named input. The build target has usually set something like this:

{
  "build": {
      "inputs": ["production", "^production"]
    },
}

The above means that our build depends on changes in its production files and those of it's dependencies.

Those inputs are usually defined in nx.json but can be also defined in your project's project.json or package.json (under nx):

{
  // ...rest of the config
  "namedInputs": {
    "default": ["{projectRoot}/**/*", "sharedGlobals"],
    "production": [
      "default",
      "!{projectRoot}/**/?(*.)+(spec|test).[jt]s?(x)?(.snap)",
      "!{projectRoot}/tsconfig.spec.json",
      "!{projectRoot}/jest.config.[jt]s",
      "!{projectRoot}/.eslintrc.json",
      "!{projectRoot}/.storybook/**/*",
      "!{projectRoot}/**/*.stories.@(js|jsx|ts|tsx|mdx)",
      "!{projectRoot}/tsconfig.storybook.json",
      "!{projectRoot}/src/test-setup.[jt]s"
    ],
}

As you can see, we specify that production input is all projects files minus the listed globs. To ignore preview.js, it needs to be added to that list. We do ignore the entire .storybook folder so I'm surprised you have this error.

Can you share your nx.json and project.json?

mandarini commented 1 year ago

Thank you @meeroslav for replying! Maybe @lukket can share a reproduction repository?

lukket commented 1 year ago

@meeroslav thanks for the explanation, that's how I understood it as well. And I'm also wondering why the default named input entry "!{projectRoot}/.storybook/**/*" doesn't seem to work.

@mandarini I created a repository with my sample code (https://github.com/lukket/nrwl-nx-18175).

As we have multiple publishable libraries with multiple storybooks and will group those anyway, this issue is not really urgent. Because, with the grouping, the problematic preview.js will move to the grouped storybook.

lukket commented 1 year ago

Hey @mandarini and @meeroslav,

Things are getting worse. I just found out that the @nx/angular:package executor also includes dependencies of tests (*.spec.ts files). I update the repository with the sample code.

If you run nx build foo, the package.json of the distributable package now contains the additional dependency @angular/platform-browser from the bar.component.spec.ts.

meeroslav commented 1 year ago

Thank you, @lukket, I will look into it now. Sorry to keep you waiting

meeroslav commented 1 year ago

By default, the @nx/angular:package executor your build is using, is using a default value for updateBuildableProjectDepsInPackageJson which is true. This means that the builder will try to update the dependencies based on the files used.

This is no longer recommended and instead you should be using the linter rule to update your dependencies - https://nx.dev/packages/eslint-plugin/documents/dependency-checks.

To disable this default behavior, you need to set updateBuildableProjectDepsInPackageJson to false in your project.json:

{
  // ...
  "targets": {
    "build": {
      "executor": "@nx/angular:package",
      "outputs": ["{workspaceRoot}/dist/{projectRoot}"],
      "options": {
        "project": "packages/foo/ng-package.json",
        "updateBuildableProjectDepsInPackageJson": false // <-- add this
      },
      // ...
    },
   // ...
}
meeroslav commented 1 year ago

If you migrate to latest version of nx you will notice that this flag is now set to true with migration (since we no longer set it to true automatically). Removing that and adding the @nx/dependency-checks rule to your eslint config would solve this for you.

lukket commented 1 year ago

@meeroslav it now works as expected. Thank you very much! The linter rule is also very helpful.

github-actions[bot] commented 1 year ago

This issue has been closed for more than 30 days. If this issue is still occuring, please open a new issue with more recent context.