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

array-type configuration differs from Visual Studio Code preview #25

Closed
EdJoPaTo opened this issue Apr 13, 2020 · 3 comments
Closed

array-type configuration differs from Visual Studio Code preview #25

EdJoPaTo opened this issue Apr 13, 2020 · 3 comments

Comments

@EdJoPaTo
Copy link

The preview in Visual Studio Code automatically uses readonly T[] over ReadonlyArray<T>.
When looking on Previews of functions and parameters I get familiar with this style of writing parameters. The current rules want me to write a different style.

I havnt looked into Visual Studio Codes Preview Logic or why they do it that way. I only want to start a discussion about it.

These two following methods are accepting the same parameter but the preview of these is the same. The first one produces this xo error:
Array type using T[] is forbidden for non-simple types. Use Array<T> instead. @typescript-eslint/array-type

export function foo(something: readonly Readonly<Something>[]): number {
  return something.length
}

export function bar(something: ReadonlyArray<Readonly<Something>>): number {
  return something.length
}

Screenshot of Visual Studio Code showing the code example

Screenshot of Visual Studio Code with a more complex parameter

What are other opinions about changing the rule array-type towards simple arrays?

@sindresorhus
Copy link
Member

I agree that the first one is nicer. The point of the rule was to prevent things like (string | boolean)[] in favor of Array<string | boolean>. Maybe the rule could be made better somehow?

@EdJoPaTo
Copy link
Author

EdJoPaTo commented Apr 15, 2020

I tend towards using extra types if unions are more complex. string | boolean might still end up in brackets for me but as soon as there are three or one of them isn't a basic type anymore the union will get its own type. That improves readability for me and I am able to use the inner type more easily.

Thinking about that maybe a rule for disallowing non plain unions would be better than a specific array-type based on their content?
For example this would be allowed:

type A = string | string[]
type B = Array<A>
type C = readonly A[]

And this would be disallowed as the unions are not at the top level

type B = Array<string | string[]>
type C = readonly (string | string[])[]

Maybe this could be configurable to allow union of simple types like this: Something<string | boolean> as they are still relatively easy to read.

There is no rule like that currently existing nor an issue suggesting such rule.
For now its just a quick thought that came up, not sure if or how much that improves things. Or maybe I'm overlooking something.

@fregante
Copy link
Member

fregante commented Aug 6, 2022

That’s what the array-simple config does exactly: https://github.com/typescript-eslint/typescript-eslint/blob/main/packages/eslint-plugin/docs/rules/array-type.md

XO’s config already uses array-simple so if it still doesn’t work correctly it should be reported to TS-ESLint. It’s not something XO can fix anyway.

@fregante fregante closed this as completed Aug 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants