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.

Sign up for GitHub

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

Jump to bottom

feat: support flat config #156

Open
wants to merge 19 commits into
base: main
Choose a base branch
from
Open

feat: support flat config #156

wants to merge 19 commits into from

Conversation

yannbf
Copy link
Member

@yannbf yannbf commented May 17, 2024 •
edited by github-actions bot

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:

  • Ran pnpm run update-all
  • Tests are updated
  • Documentation is updated

Change Type

Indicate the type of change your pull request is:

  • maintenance
  • documentation
  • patch
  • minor
  • major
📦 Published PR as canary version: 0.9.0--canary.156.da7873a.0

✨ Test out this PR locally via:

npm install [email protected]
# or 
yarn add [email protected]

@yannbf yannbf added the minor Increment the minor version when merged label May 17, 2024
@yannbf yannbf requested a review from kasperpeulen as a code owner May 17, 2024 21:00
@yannbf yannbf mentioned this pull request May 17, 2024
Closed
8 tasks
@yannbf
Copy link
Member Author

yannbf commented May 17, 2024

@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
Copy link
Member

kazupon commented May 18, 2024

@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
Copy link

@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
Copy link
Member Author

yannbf commented May 20, 2024

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
Copy link

tom-fletcher commented May 21, 2024 •
edited

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
Copy link
Member

kazupon commented May 21, 2024

@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.
image

@chohner chohner mentioned this pull request May 23, 2024
Open
@wakamsha wakamsha mentioned this pull request May 29, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@kasperpeulen kasperpeulen Awaiting requested review from kasperpeulen kasperpeulen is a code owner

Assignees
No one assigned
Labels
minor Increment the minor version when merged
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Support for the new FlatConfig format
3 participants
@yannbf @kazupon @tom-fletcher