feat: support flat config

yannbf commented 1 month ago

Closes: #135

What Changed

This is a branch off of the original PR at #152 incredibly done by @kazupon. Only reason I created a new PR is because the previous one was a fork, and therefore CI checks were not running, neither the canary release.

Original PR text:

I've supported ESLint flat configuration and eslint v9 This PR has compatible for legacy style configuration and compatible eslint API using.

So to maintain compatibility, we provide a preset with the flat namespace.

flat config example is here:

import storybook from 'eslint-plugin-storybook'

export default [
  // add more generic rulesets here, such as:
  // js.configs.recommended,
  ...storybook.configs['flat/recommended'],

  // something ...
]

This implementation is based on eslint-plugin-vue, which has several presets to support Vue 3 and Vue 2. docs is here: https://eslint.vuejs.org/user-guide/#usage

Checklist

Check the ones applicable to your change:

[x] Ran pnpm run update-all
[x] Tests are updated
[x] Documentation is updated

Change Type

Indicate the type of change your pull request is:

[ ] maintenance
[x] documentation
[ ] patch
[x] minor
[ ] major

📦 Published PR as canary version: 0.9.0--canary.156.da7873a.0

:sparkles: Test out this PR locally via: ```bash npm install eslint-plugin-storybook@0.9.0--canary.156.da7873a.0 # or yarn add eslint-plugin-storybook@0.9.0--canary.156.da7873a.0 ```

yannbf commented 1 month ago

@kazupon seems like the release step can't succeed because for some reason a pnpm lock file is modified, even though I can't really reproduce it locally. Do you have any clue?

kazupon commented 1 month ago

@yannbf Hmm 🤔 I also checked package.json and release.yml in github actions. But I can't find the cause 😅

I'd better debug it by actually looking at pnpm-lock.yaml in release.yml with actions/upload-artifact.

tom-fletcher commented 1 month ago

@yannbf - I think you were almost there with the experiment with a fix version!

Current action failure

The reason the current action is failing is that the pnpm-lock.yaml is v6, but the version of pnpm is not pinned. So the runner is using the latest version (9.1.1), which introduced a new lockfile version - and automatically upgrades the lockfile when you run pnpm install. This is why you are ending up with a modified file.

Experiment with a fix failure

You did pin the version in experiment with a fix to v8, with pnpm/action-setup, which actually prevented the first failure as it does not re-write the lockfile. But it also has the run_install setting which installs dependencies recursively, and the action was failing for a different reason.

As there are two new package.json files in the tests/integration/flat-config and tests/integrations/legacy-config directories the install created new pnpm-lock.yaml files for both of these - and the addition of these new files is what was causing the action to fail that time.

Solutions

I guess the way forward might be:

Reintroduce pnpm pinning to v8 in release.yml
Either also commit pnpm-lock.yaml files for the tests directories, or possibly.gitignore these.

(Separately, it may be worth upgrading to pnpm v9 some point in the future)

yannbf commented 1 month ago

Oh wow @tom-fletcher thank you so much for your invaluable assist! It worked wonderfully.

@kazupon the canary is ready to test!! 0.9.0--canary.156.26b630a.0

tom-fletcher commented 1 month ago

No worries @yannbf - I've spent a bit of time with pnpm and github actions recently, glad to have helped with debugging the issue. Thanks for the work yourself and especially @kazupon have done on this, looking forward to integrating this with the new eslint config on a project when it is ready. 🙂

kazupon commented 1 month ago

@yannbf Thanks! I've just tested at my day job project! That's works!

The lint itself is fine, but the type is any, so it would be nice if that could be resolved as well.

yannbf commented 1 week ago

@yannbf Thanks! I've just tested at my day job project! That's works!

The lint itself is fine, but the type is any, so it would be nice if that could be resolved as well.

Sorry for only getting back to this now. Great to know!! I asked others to try it out so we can get more confidence of merging this. Do you have any potential solution for the types?

jdelStrother commented 1 week ago

FWIW eslint-plugin-storybook@0.9.0--canary.156.26b630a.0 has been working well for us with eslint 9.5

[Update: ehh, with the small caveat it's not searching subdirectories]

bmulholland commented 2 days ago

@yannbf Re: types, I just took a look at what other plugins in my config are doing. Most just return any type, so the code here isn't far away from convention. IMO, it's not a big deal, because the eslint config doesn't support Typescript anyway, just .js extensions...

However, looks like this plugin already relies on @typescript-eslint/utils, so here's an easy fix to add types: https://github.com/SonarSource/eslint-plugin-sonarjs/blob/fc57b805ebf984646e4093b3df3d436e769795d1/src/index.ts#L92

storybookjs / eslint-plugin-storybook