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

Docs: Don’t suggest tseslint.config helper in Getting Started #8496

Closed
2 tasks done
heymartinadams opened this issue Feb 16, 2024 · 9 comments · Fixed by #8507
Closed
2 tasks done

Docs: Don’t suggest tseslint.config helper in Getting Started #8496

heymartinadams opened this issue Feb 16, 2024 · 9 comments · Fixed by #8507
Assignees
Labels
accepting prs Go ahead, send a pull request that resolves this issue documentation Documentation ("docs") that needs adding/updating

Comments

@heymartinadams
Copy link

heymartinadams commented Feb 16, 2024

Before You File a Documentation Request Please Confirm You Have Done The Following...

Suggested Changes

The “Getting Started” section currently shows a how-to using the tseslint.config helper package.
Screenshot of Getting Started section that uses tseslint.config

However, as mentioned in #8211 (comment), that was incredibly confusing to me, given that I — as do most other devs — have a number of plugins we need to connect to ESLint.

The Getting Started section isn’t the place to introduce this helper, since it creates the impression that this is the only possible way to use the typescript-eslint package. I get that you might think that the tseslint.config helper makes things easier for us — and it might, for some —, but for me it made things more confusing, and therefore more difficult.

Affected URL(s)

https://typescript-eslint.io/getting-started

@heymartinadams heymartinadams added documentation Documentation ("docs") that needs adding/updating triage Waiting for maintainers to take a look labels Feb 16, 2024
Copy link

Uh oh! @heymartinadams, the image you shared is missing helpful alt text. Check your issue body.

Alt text is an invisible description that helps screen readers describe images to blind or low-vision users. If you are using markdown to display images, add your alt text inside the brackets of the markdown image.

Learn more about alt text at Basic writing and formatting syntax: images on GitHub Docs.

@bradzacher
Copy link
Member

bradzacher commented Feb 16, 2024

My question for you would be this:

You mention "for me it made things more confusing, and therefore more difficult". What was more confusing? What was more difficult? Was it confusing to add more plugins? Was it confusing to figure out how to add rules?

More information about the troubles you ran into would help us to understand the problems and omissions from the guide so that we can improve it.


As for removing our util - I don't understand why we'd do that.

In a nutshell our getting started guide is there to show users our recommended best practice for setting up their config. In our opinion using our util is the best practice and the easiest way to write your config - so why wouldn't we show it...?

As I mentioned in this comment using our util gives you types - autocomplete for all config properties along with all of the documentation via jsdoc. Seems like a no brainier to use this over a raw, untyped array.

@bradzacher bradzacher added awaiting response Issues waiting for a reply from the OP or another party and removed triage Waiting for maintainers to take a look labels Feb 16, 2024
@JoshuaKGoldberg
Copy link
Member

An alternate idea might be to include a quick oneliner that either mentions that the helper is optional and/or links to its docs that say the same.

@pete-willard
Copy link

pete-willard commented Feb 17, 2024

I think the issue is that the average user already has their config file which exports an array of objects via module.exports. When they attempt to update the arguably most important typescript-eslint plugin they see the official docs suggest wrapping the whole config in a new helper function (is that even a correct assumption on my part?) instead of adding a new object to the array as is the new flat config convention.

I understand the added type checking benefits but it feels like an additional layer of abstraction which made me search the repository issues and ultimately leave this comment instead of just updating my existing config and moving on. I just wanted to use the recommended config plus a bunch of overrides and be on my way.

Plus now I'm getting the Cannot find module 'typescript-eslint' or its corresponding type declarations. error on the import despite meeting the node version and having moduleResolution: bundler set.
Which is to be expected bundler does not support resolution of require calls.. So it looks like compat.extends stays in my config for now.

@bradzacher
Copy link
Member

bradzacher commented Feb 17, 2024

@pete-willard The type error you describe should be fixed with the next release - Monday morning PST or try our canary release.

@heymartinadams
Copy link
Author

@bradzacher exactly what @pete-willard said.

@bradzacher
Copy link
Member

Out of curiosity did either of you see the documentation for the typescript-eslint package? Or did you only see the getting started guide?

https://typescript-eslint.io/packages/typescript-eslint

I guess there's just been a misunderstanding here - the getting started guide isn't really there to show you how to migrate a config - it's there to help someone get from 0 to linting asap.

Which is why you had a bad experience with it.
You came to the getting started page expecting to find information on how you might migrate your existing flat config from @typescript-eslint/eslint-plugin + @typescript-eslint/parser to the new typescript-eslint package. And I can understand why you'd be confused by the getting started guide - it doesn't even show a config object until the second page.

We can do better with the documentation here to try to help you get to the right page. Ideally the package doc page would be the page we want you want to get to asap. That page should include all the API docs and examples to show what you can do with the package. Ideally it would include an example of a migration.

Now that we understand the problem - we can work on improving this.

@heymartinadams
Copy link
Author

I don’t think that’s it, @bradzacher. Please hear me out, perhaps this could be important for some of your other users as well. Or maybe I’m just weird that way — I do have ADHD and easily get overwhelmed when presented with too much information and you need to take what I have to say with a grain of salt.

Some of your visitors won’t have an ESLint file and will be counting on you to guide them on how to set it up. Other visitors will have an ESLint file already set up and will be counting on you to guide them to add typescript-eslint to their existing setup.

In both cases, however, you’ll need to bank on the fact that developers will also need to add more plugins to their ESLint file — not just typescript-eslint.

That’s why I was still confused even when I saw the config object on the second page (which I came across when I tried to install typescript-eslint the first time). Here’s what I thought to myself: “Why on earth do I have to add this weird export default tseslint.config(...) to my ESLint file when that approach is so different from how the other plugins fit into the file? What’s so special about this plugin that it needs to have this special treatment?”

I’m a developer who just wants to get their ESLint config file to work — and I don’t particularly care more about the typescript-eslint plugin than about any other plugin — I just want everything to work and I want you to show me the quickest, least painful way to do that.

Fortunately, ESLint already has a convention established:

export default [
  {
    files: ['apps/web/**/*.{js,jsx,ts,tsx}'],
    languageOptions: {
      parser: [ insert your parser here],
      ...
    }
    plugins: {
      monorepo: monorepoPlugin,
      prettier: prettierPlugin,
      [ insert your plugin here]
      ...
    },
    ...
  }
]

The reason it would have been so much easier for me to see typescript-eslint embrace this convention is because it doesn’t give me more mental overhead to process; “oh, yeah, that makes sense: insert in the parser there, and put the plug over there, with the other plugins. Easy! Done! Moving on...”

I think that there’s value in convention over configuration, especially when you’re building a plugin for a shared file — one that you need to share with other plugins.

Obviously this is your service that you’re maintaining and to do with as you please (and I’m grateful for it! Thank you!) ❤️

My two cents.

@bradzacher
Copy link
Member

bradzacher commented Feb 19, 2024

I just want everything to work and I want you to show me the quickest, least painful way to do that.

To be fair - we do show that.
...tseslint.configs.recommended is the quickest, least painful way.


Based on my reading -- your comment really just reinforces what I said earlier:

You came to the getting started page expecting to find information on how you might migrate your existing flat config from @typescript-eslint/eslint-plugin + @typescript-eslint/parser to the new typescript-eslint package.

You expected something and we didn't meet that expectation -- there was no information at all about how to do this migration. So it's understandable that given you haven't seen the information you were looking for that the additional code then was confusing to you.

I.e. I imagine something like "How do I configure the parser? Does this config function do that for me? Why do I even need that - why can't they just use an array? Oh that doesn't even do what I want - it's just noise..."


The information you really were looking for is visible on the package documentation page where it shows the two lines you specifically wanted to see ('typescript-eslint': tseslint.plugin, and parser: tseslint.parser,).

However there's no clear path to get from the getting started page to package doc. There's just one link on the page and it's entirely unclear that's even a link that you would want to click on for your usecase:

unclear link

On top of that the docs on the getting started page are even misleading in their wording:

The config helper sets three parts of ESLint

  • Parser ...
  • Plugin ...
  • Rules ...

Well it doesn't do any of that, actually - it's the identity function.

Additionally if you do make it to the package docs - the docs don't make it clear what the tseslint.config is. In hindsight being terse has cost us in clarity. The page states:

The config function is a simple utility which does a few things. First and foremost it is a strictly typed function - meaning it allows you to write type-safe configs!

Which is a confusing leading sentence and doesn't actually explain anything about what it is aside from declaring its benefit.

As Josh mentioned we can improve these docs to clarify what the util is and that it's optional even if we strongly recommend it.

@bradzacher bradzacher added accepting prs Go ahead, send a pull request that resolves this issue and removed awaiting response Issues waiting for a reply from the OP or another party labels Feb 19, 2024
@bradzacher bradzacher self-assigned this Feb 19, 2024
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 28, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepting prs Go ahead, send a pull request that resolves this issue documentation Documentation ("docs") that needs adding/updating
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants