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

TypeScript Optional Response Parameters #42

Open
cnguy opened this issue Dec 31, 2017 · 2 comments
Open

TypeScript Optional Response Parameters #42

cnguy opened this issue Dec 31, 2017 · 2 comments

Comments

@cnguy
Copy link
Owner

cnguy commented Dec 31, 2017

One huge TypeScript problem (though it's ignorable) is that the response body properties are all optional.

Problem

sw2dts generates optional types by default unless the user sets the required tag in the property of a component.

Point:
  type: object
  properties:
    x:
      type: number
    y:
      type: number
  required: # <-- This
    - x
    - y

=>

export interface Point {
  x: number
  y: number
}

Without the required tags, we end up with

export interface Point {
   x?: number
   y?: number
}

The auto-generated JSON that we read from (http://www.mingweisamuel.com/riotapi-schema/openapi-3.0.0.yml) does not have the required property. However, I do not believe that is the problem of the JSON we read from. required (based on what I know) is based on parameters (path & query) for an endpoint, and not for the response classes themselves.

Manual Workaround

Workaround 1

const name: string = summoner.name // BREAKS. string != string | undefined
const name: string = summoner.name! // probably the best way (non-null assertion operator)
const name = summoner.name as string // force it to work if you know it's not undefined

Workaround 2

Manual file-editing, which is not fun.

If I forget about this and you're having issues due to bad code and run-time errors hit me up

@cnguy cnguy changed the title TypeScript Optional Response Parameteres TypeScript Optional Response Parameters Dec 31, 2017
@cnguy
Copy link
Owner Author

cnguy commented Feb 4, 2019

League API responses seem to not have optional member fields (there is no Static API anymore). It might be worth considering a pull request.

@cnguy
Copy link
Owner Author

cnguy commented Mar 8, 2019

MingweiSamuel/riotapi-schema#7

Will revert the typings as it is unsafe and I don't want to go through every response to determine what fields are missing in which situation until I have a lot of free time. Body param DTOs will gain their optional params, which is nice (requires casting though), while response DTOs will still use optional fields.

cnguy added a commit that referenced this issue Mar 16, 2019
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

1 participant