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

Suport ESLint v9 #157

Open
bmulholland opened this issue May 30, 2024 · 16 comments
Open

Suport ESLint v9 #157

bmulholland opened this issue May 30, 2024 · 16 comments

Comments

@bmulholland
Copy link

Separately from flat config, ESLint v9 dropped several deprecated APIs, including context.getScope, which this plugin uses. Those need to be replaced.

@bmulholland
Copy link
Author

bmulholland commented Jun 4, 2024

@Sv443 It is not. Flat config was available in ESLint 8, and is optional in ESLint 9. My issue specifically says "separately from flat config," which I guess you missed?

@maxpatiiuk
Copy link

This plugin also depends on a very old version of @typescript-eslint/utils ^5.62.0.
That version of @typescript-eslint/utils does not support ESLint 9, which causes yarn dependency resolution to fail

@yannbf
Copy link
Member

yannbf commented Jul 2, 2024

Hey @maxpatiiuk seems like the typescript utils only support ESLint v9 in an alpha version, which is a bummer.

I'm honestly not sure if it's possible to support both v8 and v9 of ESLint..

@maxpatiiuk
Copy link

maxpatiiuk commented Jul 2, 2024

I'm honestly not sure if it's possible to support both v8 and v9 of ESLint..

Yes, it's a tricky situation.

Options:

  • The v8 version of @typescript-eslint/utils supports both ESLint 8 and 9: https:/typescript-eslint/typescript-eslint/blob/9dce771b8dd4edae252af388b3767b95dfb007f3/packages/utils/package.json#L72
  • You can use a simple bundler like tsup to bundle-in the parts of @typescript-eslint/utils that are needed by your library. Then, you package will declare @typescript-eslint/utils as a devDependency rather than a dependency, avoiding the package manager complaints
  • Wait 1-2 months for typescript-eslint v8 to have a stable release and to be more widely used. Until then, publish the eslint v9 version of eslint-plugin-storybook as a separate npm tag

edit: lack of eslint 9 support in this package is not a deal breaker for us (we can temporary disable it). a few other packages we have a bigger blockers, and we must wait as the ecosystem churns a bit

@reduckted
Copy link

I'm honestly not sure if it's possible to support both v8 and v9 of ESLint..

Something that might influence the decision: ESLint v8 is EOL in less than two months. https://eslint.org/version-support/

The best solution might be to drop support for v8. If anyone needs to use ESLint v8, they can use the current version of this package (0.8.0) instead of the latest version.

@tony19
Copy link

tony19 commented Aug 14, 2024

Hey @maxpatiiuk seems like the typescript utils only support ESLint v9 in an alpha version, which is a bummer.

@yannbf If the alpha status is the dealbreaker here, typescript-eslint@8 is no longer in alpha. latest is currently v8.1.0.

@re-taro
Copy link

re-taro commented Sep 25, 2024

Hi @yannbf (cc: @shilman ) !

I am working on a solution to this issue! I have come up with the following solution, which we hope you will review and consider.

Assumption

Because of the premise in the peerDependencies here, not all versions of eslint supported by eslint-plugin-storybook can be supported.

https:/typescript-eslint/typescript-eslint/blob/896c7312dfe8374312df9404cb11db8378cd091c/packages/utils/package.json#L71-L73

I then propose the following solution, which I hope you will consider with understanding.

Suggestion

Release a version of eslint-plugin-storybook with fixed peerDependencies

Release a version of eslint-plugin-storybook with fixed peerDependencies. This version is intended for users of eslint v6 ~ v8.56 or earlier. (Since they are using the version of eslint that is or will be EOL, it is assumed that they do not have to deal with any bug fixes).

  "peerDependencies": {
    "eslint": ">=6 <8.57"
  }

Migration to v8 based on @typescript-eslint release notes

The official @typescript-eslint blog has a description of what you should change, so have a look at it and do your own modifications.

https://typescript-eslint.io/blog/announcing-typescript-eslint-v8#developer-facing-changes

(OPTIONAL) Create compat utils

There is a way to create compat utils like the following, but this has the drawback that the eslint-plugin-storybook page knows the type definitions of @typescript-eslint, so it is not considered a good solution from a maintainability and extensibility point of view.

import type { TSESLint, TSESTree } from '@typescript-eslint/utils'

declare module '@typescript-eslint/utils/dist/ts-eslint/Rule' {
  export interface RuleContext<TMessageIds extends string, TOptions extends readonly unknown[]> {
    /**
     * The filename associated with the source.
     */
    filename: string

    /**
     * A SourceCode object that you can use to work with the source that
     * was passed to ESLint.
     */
    sourceCode: Readonly<TSESLint.SourceCode>
  }
}

declare module '@typescript-eslint/utils/dist/ts-eslint/SourceCode' {
  export interface SourceCode {
    /**
     * Returns the scope of the given node.
     * This information can be used track references to variables.
     * @since 8.37.0
     */
    getScope(node: TSESTree.Node): TSESLint.Scope.Scope
  }
}

export const getFilename = (
  context: Readonly<TSESLint.RuleContext<string, readonly unknown[]>>
) => {
  return context.filename ?? context.getFilename()
}

export const getSourceCode = (
  context: Readonly<TSESLint.RuleContext<string, readonly unknown[]>>
) => {
  return context.sourceCode ?? context.getSourceCode()
}

export const getScope = (
  context: Readonly<TSESLint.RuleContext<string, readonly unknown[]>>,
  node: TSESTree.Node
) => {
  return getSourceCode(context).getScope?.(node) ?? context.getScope()
}

And finally...

Sorry to send you a suggestion with a mentions of how busy I am. I wanted to send it in the form of a PR message, but I also wanted to discuss community policy and direction, so it ended up in the form of a suggestion.

@michaelfaith
Copy link

Release a version of eslint-plugin-storybook with fixed peerDependencies. This version is intended for users of eslint v6 ~ v8.56 or earlier. (Since they are using the version of eslint that is or will be EOL, it is assumed that they do not have to deal with any bug fixes).

  "peerDependencies": {
    "eslint": ">=6 <8.57"
  }

This would be a breaking change on its own if it isn't inclusive of 8.57. There are undoubtedly people using the plugin today with eslint 8.57

@re-taro
Copy link

re-taro commented Sep 25, 2024

This would be a breaking change on its own if it isn't inclusive of 8.57. There are undoubtedly people using the plugin today with eslint 8.57

That could easily happen. We think the key is the timing of the release.

  1. release with modified peerDependencies
  2. release with @typescript-eslint v8 support

I thought it would be better to release them at the same time and have an official announcement. (Please note that this is a user's opinion, not an official one)

@michaelfaith
Copy link

But releasing a breaking change in 1. as a non-major version change is a bit of a non-starter. Why not just include 8.57 in there?

@re-taro
Copy link

re-taro commented Sep 25, 2024

Hmmm, yes, indeed.

The reason I'm using the scope of my proposed peerDependencies is because I see it as going up to @typescript-eslint v8 peerDependencies or earlier. I think there is enough room for consideration here.

@yannbf
Copy link
Member

yannbf commented Oct 4, 2024

Hey everyone! Sorry for taking long to solve this. We've been quite busy with multiple projects.
We are going to focus on sorting this out in the coming weeks, so please be a little more patient and we'll get there!

@re-taro thanks for your insights. There is a PR here that tries to support ESLint v9 also achieve backwards compatibility while introducing support for flat config, please stay tuned:
#156

There is already a canary that you can try out today: 0.10.0--canary.156.408aed4.0

Although the canary has been confirmed by many people that it works as intended, there is still more work to be done.

@michaelfaith
Copy link

@yannbf thanks for the update. Is there anything I can do to help?

@Cyberboss
Copy link

I think that canary doesn't declare the peer deps correctly via teslint? Still erroring for meImage

@re-taro
Copy link

re-taro commented Oct 6, 2024

@yannbf

@re-taro thanks for your insights. There is a PR here that tries to support ESLint v9 also achieve backwards compatibility while introducing support for flat config, please stay tuned:
#156

Thanks for sharing the PR! I'm sorry, I thought this PR was only for flat config, but I was wrong!

@re-taro
Copy link

re-taro commented Oct 6, 2024

I think that canary doesn't declare the peer deps correctly via teslint? Still erroring for meImage

Hi @Cyberboss

Probably because the version of @typescript-eslint/utils it depends on is the one before eslint v9 support. I suspect that 0.10.0--canary.156.408aed4.0 does not yet support the breaking changes in the @typescript-eslint/utils API.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Empathy Backlog
Development

No branches or pull requests

8 participants