feat: support for enum in type assignments

ericpyle commented 4 months ago

It would be nice if we could get the same prettified tooltip support for enums in type assignments:

mylesmmurphy commented 4 months ago

Hey @ericpyle! Thanks for bringing this to my attention, I will take a look into this. However, does it work if you remove the & (), and just assign directly to the union? Either way I will look into this! I.e. type EventTypeForAPI = BasicEventType | PostableEventType | NoteEventType

ericpyle commented 4 months ago

Hey @ericpyle! Thanks for bringing this to my attention, I will take a look into this. However, does it work if you remove the & (), and just assign directly to the union? Either way I will look into this! I.e. type EventTypeForAPI = BasicEventType | PostableEventType | NoteEventType

Good suggestion. However, it didn't change anything.

I think the issue is that tooltips for enums themselves don't show the enum details.

mylesmmurphy commented 4 months ago

Looking into the code, it does look like the extension is incorrectly ignoring displaying the hover information. This is a bug with how I'm doing the check to ignore redundant information that I will fix in the next update 👍

If you open the type preview panel under the menu and click on the type, do you see what you'd expect? It does appear to prettify the enum there, but I'm not sure if it's in a format you'd necessarily expect.

mylesmmurphy commented 4 months ago

For example, when I try the following snippet:

enum TestEnum {
    A = 'a',
    B = 'b'
}
enum TestEnum2 {
    C = 'c',
    D = 'd'
}
enum TestEnum3 {
    E = 'e',
    F = 'f'
}
type TestType = TestEnum | TestEnum2 | TestEnum3

Would you instead expect to see this?: type TestType = 'a' | 'b' | 'c' | 'd' | 'e' | 'f'

The tricky part with that is with how enums work in TS, because the following works: const test: TestType = TestEnum.A;

but this is invalid: const test: TestType = 'a';

mylesmmurphy commented 4 months ago

Or

type TestType =
    | TestEnum.A
    | TestEnum.B
    | TestEnum2.C
    | TestEnum2.D
    | TestEnum3.E
    | TestEnum3.F

ericpyle commented 4 months ago

A side question, for the formatting of the preview, would you expect it to be displayed in one line, or broken over multiple? type TestType = TestEnum.A | TestEnum.B | TestEnum2.C | TestEnum2.D | TestEnum3.E | TestEnum3.F

Or
type TestType =
    | TestEnum.A
    | TestEnum.B
    | TestEnum2.C
    | TestEnum2.D
    | TestEnum3.E
    | TestEnum3.F

Good question. Breaking it up over multiple lines might make it more human readable, so that's my current preference if that's easy enough and doesn't create technical debt for you

ericpyle commented 4 months ago

Would you instead expect to see this?: type TestType = 'a' | 'b' | 'c' | 'd' | 'e' | 'f'

My use case is to make it easy to copy and paste for api reference. As such, the final strings are important to me, but I can just copy and paste each enum into the documentation to show what the string values are. I think for most people just using the enum keys rather than their values is what people would want to see. Thoughts?

mylesmmurphy commented 2 months ago

Hey @ericpyle - wanted to apologize for the delay on this. I am picking this work back up soon and plan to have it addressed in the next release.

I think for most people just using the enum keys rather than their values is what people would want to see.

I completely agree, I will implement it this way.

Breaking it up over multiple lines might make it more human readable, so that's my current preference if that's easy enough and doesn't create technical debt for you

This ended up being more difficult to implement than I originally intended, but definitely should be do-able in the future. I will add a backlog item to address it in the future!

ericpyle commented 2 months ago

No problem. All sounds good to me. Thank you!

On Wed, Apr 17, 2024, 10:19 PM Myles Murphy @.***> wrote:

Hey @ericpyle https://github.com/ericpyle - wanted to apologize for the delay on this. I am picking this work back up soon and plan to have it addressed in the next release.

I think for most people just using the enum keys rather than their values is what people would want to see.

I completely agree, I will implement it this way.

Breaking it up over multiple lines might make it more human readable, so that's my current preference if that's easy enough and doesn't create technical debt for you

This ended up being more difficult to implement than I originally intended, but definitely should be do-able in the future. I will add a backlog item to address it in the future!

— Reply to this email directly, view it on GitHub https://github.com/mylesmmurphy/prettify-ts/issues/15#issuecomment-2062919338, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAISZPL2FGUGSFKMRG4RAMDY543TRAVCNFSM6AAAAABDKGEMQ2VHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDANRSHEYTSMZTHA . You are receiving this because you were mentioned.Message ID: @.***>

--

This email and any attachments are confidential and intended only for those to which it is addressed. If you have received this message in error please inform the sender and delete the email. The content of this message belongs to the sender and the views expressed do not necessarily reflect the views of United Bible Societies.

United Bible Societies Association, a company limited by guarantee. Registered in England and Wales No 2264875. Registered Charity No 800058 Registered Office: Stonehill Green, Westlea, Swindon, Wiltshire, SN5 7PJ, England.

mylesmmurphy commented 1 month ago

This issue has been resolved in the latest release v0.0.18 👍

Note: enums will not get expanded when referenced inside an object. Unfortunately adding that becomes quite complicated. However, when hovering in your example, you will now see all possible enum values 😄

mylesmmurphy / prettify-ts

feat: support for enum in type assignments #15