Nx: feat: @nrwl/storybook: stories are not linted - it's on purpose or a bug?

Created on 24 Apr 2020  路  10Comments  路  Source: nrwl/nx

Expected Behavior

I see two options how to approach this:

  • use tsconfig.json as lint entry point within architect lint target
"lint": {
          "builder": "@nrwl/linter:lint",
          "options": {
            "linter": "eslint",
            "config": "libs/react-one/.eslintrc",
            "tsConfig": [
-              "libs/react-one/tsconfig.lib.json",
-              "libs/react-one/tsconfig.spec.json"
+              "libs/react-one/tsconfig.json"
            ],
            "exclude": ["**/node_modules/**", "!libs/react-one/**"]
          }
        },
  • generate tsconfig.stories.json when storybook:init is executed and update particular project workspace.json accordingly
"lint": {
          "builder": "@nrwl/linter:lint",
          "options": {
            "linter": "eslint",
            "config": "libs/react-one/.eslintrc",
            "tsConfig": [
              "libs/react-one/tsconfig.lib.json",
              "libs/react-one/tsconfig.spec.json"
+              "libs/react-one/tsconfig.stories.json"
            ],
            "exclude": ["**/node_modules/**", "!libs/react-one/**"]
          }
        },

with tsconfig.stories.json

{
  "extends": "./tsconfig.json",
  "compilerOptions": {
    "outDir": "../../dist/out-tsc",
    "types": [
      "node"
    ]
  },
  "include": [
    "**/*.stories.ts",
    "**/*.stories.js"
  ]
}

Current Behavior

  • .stories are not linted

image
image

Context

>  NX  Report complete - copy this into the issue template

  @nrwl/angular : Not Found
  @nrwl/cli : 9.2.3
  @nrwl/cypress : 9.2.3
  @nrwl/eslint-plugin-nx : 9.2.3
  @nrwl/express : Not Found
  @nrwl/jest : 9.2.3
  @nrwl/linter : 9.2.3
  @nrwl/nest : Not Found
  @nrwl/next : Not Found
  @nrwl/node : 9.2.3
  @nrwl/react : 9.2.3
  @nrwl/schematics : Not Found
  @nrwl/tao : 9.2.3
  @nrwl/web : 9.2.3
  @nrwl/workspace : 9.2.3
  typescript : 3.8.3
angular react storybook bug

All 10 comments

WDYT folks? @juristr @jaysoo

Hi @Hotell ! :)

I am not sure I could reproduce your issue. Let me tell you the steps I followed and you can tell me if this is the issue you encountered.

Steps that I followed:

  1. npx create-nx-workspace@latest test-linting and _choose react_
  2. nx g @nrwl/react:lib ui-test
  3. yarn add --dev @nrwl/storybook
  4. nx g @nrwl/react:storybook-configuration --name=ui-test and _choose yes to all prompts_
  5. nx run ui-test:lint
  6. All files pass linting
  7. Mess up test-linting/libs/ui-test/src/lib/ui-test.tsx file
  8. nx run ui-test:lint again
  9. Get some errors and warnings
  10. nx run ui-test:lint --fix
  11. Fixed one error

So, it seems to me that the .stories.tsx files do get linted. Is this what you were referring to, or is it something else?

Screenshot 2020-07-23 at 4 07 27 PM

Hey @mandarini ,

  • you need to create a story file (foo.stories.tsx) and break some lint rule within that story.
  • because story files are not included within linting there are no exposed issues

Please check again my report.

Thx

Hi @Hotell !

You are absolutely right, I'm silly! I mean to show you that .stories.tsx files get linted, but in my second run to produce the screenshots and the steps that I posted above, I got confused and referred you to the .tsx files. I should have double checked my reply!

So, let me try again!

I am not sure I could reproduce your issue. Let me tell you the steps I followed and you can tell me if this is the issue you encountered.

Screenshot 2020-07-24 at 10 13 16 AM

Steps that I followed:

  1. npx create-nx-workspace@latest test-linting and choose react
  2. nx g @nrwl/react:lib ui-test
  3. yarn add --dev @nrwl/storybook
  4. nx g @nrwl/react:storybook-configuration --name=ui-test and choose yes to all prompts
  5. nx run ui-test:lint
  6. All files pass linting
  7. Mess up test-linting/libs/ui-test/src/lib/ui-test.stories.tsx file
  8. nx run ui-test:lint again
  9. Get some errors and warnings
  10. nx run ui-test:lint --fix
  11. Fixed one error

So, it seems to me that the .stories.tsx files do get linted. Is this what you were referring to, or is it something else? Please see screenshot attached!

hey no worries !

So the problem is, that storybook schematic adds following glob to exclude pattern (I'd say it added "**/*.stories.tsx" back when I reported this but I might be wrong ofc) :

"**/*.stories.ts",
"**/*.stories.js"

That means whilst react libs/apps have most of the time modules with extension tsx which will be passed down to both tsc and nrwl/lint builder, but for angular projects stories won't get linted.

So I think one of following should be applied:

1.

  • "fix" current generator and add "**/*.stories.tsx" (if react project - this one https://github.com/nrwl/nx/blob/master/packages/storybook/src/schematics/configuration/configuration.ts#L88-L92) to exclude (to make it consistent) and create tsconfig.stories.json with include with "**/*.stories.ts", "**/*.stories.tsx","**/*.stories.js"
  • register this config within solution file
  • add tsconfig.stories.json config to workspace lint builder config (as described in my original proposal).


    • same as first two steps in 1)

  1. add only solution tsconfig.json to every lint builder config

If agreed within team I can work on this 馃檶

Hey @Hotell 馃憢 . Just discussed the options with @mandarini. We would go with option 1, so

1) fix the matching of the stories in the react storybook setup as you correctly pointed out
2) add the storybook tslint to the linter configuration in angular.json / workspace.json. But instead of generating a new one we could directly use the one in the .storybook/tsconfig.json which is generated inside the lib that has storybook support.

Would be cool if you could provide a PR on this 馃檪

Ha, I didn't thought about that one (.storybook/tsconfig.json) great idea!

Question: will lint builder properly resolve provided relative path? "include": ["../src/**/*"] see source

Would be cool if you could provide a PR on this

I'll try to send PR till end of this week 馃

thanks folks!

Question: will lint builder properly resolve provided relative path? "include": ["../src/*/"] see source

馃 good question. I did a quick try by referencing the .storybook/tsconfig and it seemed to work. But guess you'll have to give it a try and test some edge-cases to be 100% sure :)

I'll try to send PR till end of this week 馃

cool 馃帀. Just let me or @mandarini know if you have any doubts or questions.

all right folks, here it is https://github.com/nrwl/nx/pull/3455 馃

Was this page helpful?
0 / 5 - 0 ratings