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

fix: required query parameters with content #683

Merged

Conversation

einartryggvi
Copy link
Contributor

Status

READY

Description

Support required query parameters with content instead of schema along with tests that verify that code generation for required/non-required and with/without default generates types as optional correctly.

Currently client generation fails for required query parameters with content, but works for required query parameters with schema.

The error is TypeError: Cannot read properties of undefined (reading 'default') since the schema variable is undefined when you specify content.

# Works
queryParamWithContentNotRequired:
  name: queryParamWithContentNotRequired
  in: query
  required: false
  content:
    application/json:
      schema:
        type: object
        properties:
          prop1:
            type: string
          prop2:
            type: string

# Works
queryParamWithSchemaNotRequired:
  name: queryParamWithSchemaNotRequired
  in: query
  required: false
  schema:
    type: object
    properties:
      prop1:
        type: string
      prop2:
        type: string

# Works
queryParamWithSchemaRequired:
  name: queryParamWithSchemaRequired
  in: query
  required: true
  schema:
    type: object
    properties:
      prop1:
        type: string
      prop2:
        type: string

# Doesn't work
queryParamWithContentRequired:
  name: queryParamWithContentRequired
  in: query
  required: true
  content:
    application/json:
      schema:
        type: object
        properties:
          prop1:
            type: string
          prop2:
            type: string

@vercel
Copy link

vercel bot commented Nov 27, 2022

Someone is attempting to deploy a commit to a Personal Account owned by @anymaniax on Vercel.

@anymaniax first needs to authorize it.

@einartryggvi
Copy link
Contributor Author

@anymaniax Any chance this could be merged soon?

@anymaniax
Copy link
Collaborator

Hello @einartryggvi, I maybe miss something but I don't see any fix in your code.

@einartryggvi
Copy link
Contributor Author

@anymaniax true, it's not obvious from the diff! I could have been clearer in the description of where the error occurs in the code. Sorry about that!

So the schema variable is being used elsewhere in the function, and since that isn't fallbacking on content['application/json'] it crashes.

There are a few lines that attempt to access schema.default, mainly this one: https://github.com/anymaniax/orval/blob/master/packages/core/src/getters/query-params.ts#L80

@anymaniax
Copy link
Collaborator

ok gotcha thanks!

@anymaniax anymaniax merged commit 98e311a into orval-labs:master Jan 6, 2023
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

Successfully merging this pull request may close these issues.

2 participants