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

String field cannot be optional. #122

Open
yuto-katsuragi opened this issue Apr 11, 2019 · 3 comments
Open

String field cannot be optional. #122

yuto-katsuragi opened this issue Apr 11, 2019 · 3 comments

Comments

@yuto-katsuragi
Copy link

Description

Primitive Types can be optional.
but string field can not be optional.

Repro steps

  1. Step A
    provide following this in swagger yaml
      number_value:
        type: number
        nullable: true
      string_value:
        type: string
        nullable: true
  1. Step B

read property by SwaggerProvider

type Provider = SwaggerProvider<schema, PreferAsync = true>
let client = Provider.Client()
let result = client.Operation() |> Asnyc.RunSynchronously

let numberValue = result.NumberValue // ==> the type is option float
let stringValue = result.StringValue // ==> the type is string

Expected behavior

nullable string field should be Option type.

Actual behavior

nullable string field is not Option type.

Known workarounds

  • null checking

Related information

  • Operating system
    Windows 10
  • NuGet Version
    0.10.0-beta05
  • .NET Framework 4.7.2
@sergey-tihon
Copy link
Member

This is by design. string in .net is nullable type.
I do not see benefits of generating Option<string>

So, technically speaking, the issue is that we model

string_value:
        type: string
        nullable: false

as string that can be null in runtime.
But there is no easy way to provide not nullable string.

@yuto-katsuragi
Copy link
Author

yuto-katsuragi commented Apr 11, 2019

There are several facts.

  1. string in .NET is nullable type
  2. string and null are different types in JSON

It can be understood that null can not be removed. (by 1.)
but I see benefits as reflect intention as option . (by 2.)

Similar situations.

SQL Server has intended to be null (NULL Column)
and Popular Type Provider takes care of it.

https://github.com/fsprojects/FSharp.Data.SqlClient

this library type following.
varchar NULL in DDL -> string option in .NET

I think so.
Even if can not remove null, can reflect intention as option

Honestly, there is a problem of preference.
But I think the string option is worth it.

@weebs
Copy link

weebs commented Jun 13, 2019

I came here to post about the same thing, it'd be really helpful to have this as an option like we do with IgnoreOperationId, at least in my case.

I have a RESTful API serving data from SQL where some columns are nullable text while others are not. A lot of these endpoints serve basic CRUD operations, and so I made a generic editor that uses reflection on the client generated by SwaggerProvider to save myself time from coding each one by hand. Unfortunately I don't really see a good way to determine which strings are actually nullable and which are not, so I can't easily communicate that through the generic editors (and pretty much every one has a mixture of non-nullable and nullable strings)

While .NET does have nullable strings, if the spec is communicating that they're non-nullable, I think it makes sense to try and conform to the spec as close as possible with an option type.

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