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

Replace ApiDocMemberDetails union with a record #933

Open
MangelMaxime opened this issue Jul 14, 2024 · 2 comments
Open

Replace ApiDocMemberDetails union with a record #933

MangelMaxime opened this issue Jul 14, 2024 · 2 comments

Comments

@MangelMaxime
Copy link

ApiDocMemberDetails is currently described as a single case DUs.

type ApiDocMemberDetails =
| ApiDocMemberDetails of
usageHtml: ApiDocHtml *
paramTypes: (Choice<FSharpParameter, FSharpField> * string * ApiDocHtml) list *
returnType: (FSharpType * ApiDocHtml) option *
modifiers: string list *
typars: string list *
extendedType: (FSharpEntity * ApiDocHtml) option *
location: string option *
compiledName: string option

This makes accessing the information in it not easy because tuples doesn't expose a named property to accessing the different values. The user is responsible, for manually naming each variables.

let (ApiDocMemberDetails(usageHtml,
                         paramTypes,
                         returnType,
                         modifiers,
                         typars,
                         baseType,
                         location,
                         compiledName)) =
    entity.Details

I propose to change this type to a record which will exposed named properties making it easier for consumer to use the type.

type ApiDocMemberDetails =
    {
        UsageHtml: ApiDocHtml 
        ParamTypes: (Choice<FSharpParameter, FSharpField> * string * ApiDocHtml) list 
        ReturnType: (FSharpType * ApiDocHtml) option 
        Modifiers: string list 
        Typars: string list 
        ExtendedType: (FSharpEntity * ApiDocHtml) option 
        Location: string option 
        CompiledName: string option
    }

// Usage
let details : ApiDocMemberDetails = // ...

details.ParamTypes 
// etc.
@nojaf
Copy link
Collaborator

nojaf commented Aug 5, 2024

I see only one usage of this, is this really worth the breaking change?
How many times does this bother you?

@MangelMaxime
Copy link
Author

I am not using it very often, but when doing so it is a pain to manually find the name of the properties.

A record is also more robust because you can make it evolve without breaking.

I leave it up to you (maintainers) to decide if making the switch is worth it or not. If not feel free to close :)

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

2 participants