microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
100.95k stars 12.47k forks source link

React props in typescript are displayed in alphabetical order instead of showing component props at the top when running "trigger suggest" #52080

Open MichaelHindley opened 1 year ago

MichaelHindley commented 1 year ago

Does this issue occur when all extensions are disabled?: Yes

Steps to Reproduce:

  1. git clone reproduction repo at https://github.com/MichaelHindley/vscode-react-props-alphabetical
  2. start vscode with --disable-extensions flag
  3. run npm i to install dependencies
  4. go to Line 25 in App.tsx, place curser inside the Menu component opening tag
  5. run the "Trigger suggest" command
  6. Observe the props being sorted in alphabetical order instead of showing the components own props at the top image

What should happen instead is that the component scoped variables from "MenuProps" should be at the top, rather than being alphabetically sorted. The alphabetical sorting is a very poor developer experience. When triggering suggestions in a react component, the most common scenario by far is to see which props the component type has, which vscode is aware of.

image

Notice the equivalent of "trigger suggest" in Intellij, showing the components props above all the default html props;

image
zardoy commented 1 year ago

The alphabetical sorting is a very poor developer experience.

This is so true, I was trying to proof the same in https://github.com/microsoft/TypeScript/issues/49012 and I think you're second who thinks so. This is especially important for React because of aria-* attributes. However, TypeScript can easily loose original sorting of properties in some cases, so its easier to just do alphabetical sorting in last step and your Menu component from antd is just ideal example for this (see below).

Also, this issue has a few incorrectness:

  1. Its incorrect to compare VS Code and WebStorm with Machine-Learning assisted sorting enabled, which make sorting based on previous usages, you'd probably get the same results if vscode alternative extension - IntelliCode was smarter.

Notice the equivalent of "trigger suggest" in Intellij, showing the components props above all the default html props

Its not quite true. Props are declared in the following order: [theme, inlineIndent, _internalDisableMenuItemTitleTooltip, items], [prefixCls, rootClassName, items, children, disabled, ...], htmlProps so by this logic class name should be showed after all these, so again its just webstorm opinionated sorting + "ai sorting"

You can easily make your VS Code extension that would remember most-used properties on specific components from your personal usages.


I have extension that fixes this issue, so in some simple cases like this I can restore original sorting with vscode plugin (with setting enabled):

const MyComponent: React.FC<{ myProp?: boolean } & React.ComponentProps<'div'>> = () => <div />;
<MyComponent /**/ /> // key, myProp, ref, other div props

However in more complicated cases, for example with just using Omit the sorting is lost.

interface BaseProps extends React.ComponentProps<'div'> {
    // those are not on top anymore
    c?
    d?
}

interface Props extends Omit<BaseProps, 'foo'> {
    b?
    a?
}

declare const MyComponent: React.FC<Props>;
<MyComponent />

However I use now also alternative solution: list all props declared in @types/react after all other props. Is this the desired behavior?

MichaelHindley commented 1 year ago

There is no difference with the machine-learning suggestions and without in this case, showing the components own props on top has been default in Intellij for as long as I can remember React support existing, which is why when working with VSCode it instantly stands out as a poor developer experience. See screenshot below with ML learning turned off; image

Its not quite true. Props are declared in the following order: [theme, inlineIndent, _internalDisableMenuItemTitleTooltip, items], [prefixCls, rootClassName, items, children, disabled, ...], htmlProps so by this logic class name should be showed after all these, so again its just webstorm opinionated sorting + "ai sorting"

Not quite sure if I'm defining this correct, but when I say "components own props" it means the props that the component itself defines, for the "Menu" component it refers to "MenuProps". Here is how it looks like to me and how Intellij correctly shows the components own props without ML assisted suggestions: image

These are the props from the "MenuProps" type, which the "Menu" component defines, other props start to show up in sorting after these props that the component itself has defined. What I mean is that sorting should start with the props that the component itself defines, any other props should come after that. This behavior can be controlled by triggering different type of suggestions on-press.

starball5 commented 1 year ago

Related on Stack Overflow: TypeScript React props priority when using IntelliSense in VS Code

zardoy commented 1 year ago

Thank you for linking! However, I see some interesting parts

So, as far as I know, at the time of this writing, there is no mechanism for a user to configure certain suggestions to have a higher priority.

Actually, I've thought about adding sort jsdoc support but specifying the sorting manually every time would be annoying, instead, you can move the property to the top in the interface and use the aforementioned extension with the following setting: "tsEssentialPlugins.fixSuggestionsSorting": true,

So with the component from the StackOverflow question, you should see the property tag displayed on top.

And also I do not recommend using editor.suggest.localityBonus see this issue for more. The same thing applies for editor.suggestSelection: recentlyUsed as well.

gbrocha commented 1 year ago

Is there some update here? I think it's a common issue

AnandaArya commented 8 months ago

please i need some update regarding this issue

are there any way to sort this number? i would like to have 1,2,3,4,5,6,7,8,9 image

zardoy commented 8 months ago

please i need some update regarding this issue

What kind of update do you expect?

What about number properties sorting: i think sorting of numbers can actually be forced with the language service, but I think while introducing sorting changes would be logical for most of us it can also introduce unexpected behaviors for others (also keep in mind there are edge cases). On the other hand TS plugins are meant to fine-tune DX and can easily solve these problems, really recommend to try them out.

Michota commented 4 months ago

Bump!

zardoy commented 4 months ago

Bump!

You want something?