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

MSW: Enum types should be generated "as const" in mocks #960

Closed
ClementLevesque opened this issue Oct 6, 2023 · 13 comments · Fixed by #1035
Closed

MSW: Enum types should be generated "as const" in mocks #960

ClementLevesque opened this issue Oct 6, 2023 · 13 comments · Fixed by #1035
Assignees
Labels
bug Something isn't working
Milestone

Comments

@ClementLevesque
Copy link
Contributor

ClementLevesque commented Oct 6, 2023

👋 It seems that enum types in mocks are not generated "as const", therefore mocks do not satisfy routes generated types (string vs "string" as const).

What are the steps to reproduce this issue?

  • Have a schema with a route that returns an object with an enum type: "properties":{"type":{"type":"string","enum":["value"]}...
  • Generate a mock for such a route

What happens?

The mock generated becomes :

type: faker.helpers.arrayElement(['crop']) // type is a string but should be of type: 'crop'

What were you expecting to happen?

type: faker.helpers.arrayElement(['crop'] as const)

Any other comments?

It might be fixed with the following (not tested) :

value: `faker.helpers.arrayElements(Object.values(${enumValue}))`,

          value: `faker.helpers.arrayElements(Object.values(${enumValue}) as const)`,

https://github.com/anymaniax/orval/blob/ed1261af7ff773ca1338302e313ac8f02c3da00c/packages/msw/src/getters/scalar.ts#L197C1-L197C60

        value = `faker.helpers.arrayElement(${enumValue}) as const`;

What versions are you using?

Operating System: MacOs
Package Version: 6.17.0

Thanks for your help 🙏 ❤️

@melloware melloware added the bug Something isn't working label Nov 3, 2023
@melloware melloware changed the title Enum types should be generated "as const" in mocks MSW: Enum types should be generated "as const" in mocks Nov 3, 2023
@melloware
Copy link
Collaborator

@Will-Mann-16 this looks like an easy one to verify and fix for MSW?

@Will-Mann-16
Copy link
Collaborator

I'll take a look.

@Will-Mann-16
Copy link
Collaborator

Will-Mann-16 commented Nov 7, 2023

@ClementLevesque looking at this it seems to work correctly, I'm getting a union type passed through correctly and it uses Object.values from the get go. This is the result I get and the openapi config I use. Can you provide a spec where it doesn't work as expected?

CleanShot 2023-11-07 at 11 33 23@2x
openapi: 3.0.0
info:
  title: ""
  description: Generated by astra
  contact: {}
  license:
    name: ""
  version: ""
servers:
  - url: http://localhost:8000/
paths:
  /json:
    get:
      description: GetJSON is a test function
      parameters:
        - name: hello
          in: header
          schema:
            type: string
      responses:
        "200":
          description: ""
          headers:
            x-google-api-key:
              schema:
                type: string
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/main.ResponseStruct"
        "400":
          description: ""
          headers:
            x-google-api-key:
              schema:
                type: string
          content:
            application/json:
              schema:
                $ref: "#/components/schemas/example_route_in.InResponseStruct"
components:
  schemas:
    database_sql.NullString:
      type: object
      properties:
        String:
          type: string
        Valid:
          type: boolean
      description: |-
        NullString represents a string that may be null.
        NullString implements the Scanner interface so
        it can be used as a scan destination:

        	var s NullString
        	err := db.QueryRow("SELECT name FROM foo WHERE id=?", id).Scan(&s)
        	...
        	if s.Valid {
        	   // use s.String
        	} else {
        	   // NULL value
        	}
    example_route_in.InResponseStruct:
      type: object
      properties:
        hello:
          type: string
      description: InResponseStruct is a test struct
    main.ResponseStruct:
      type: object
      properties:
        enum-int:
          $ref: "#/components/schemas/main.TestEnumInt"
        enum-string:
          $ref: "#/components/schemas/main.TestEnumString"
        map:
          type: object
          additionalProperties:
            type: string
        not-required:
          type: integer
          format: int32
        required:
          type: string
        separate:
          type: array
          items:
            $ref: "#/components/schemas/example_route_in.InResponseStruct"
        test:
          $ref: "#/components/schemas/database_sql.NullString"
        time:
          $ref: "#/components/schemas/time.Time"
      description: ResponseStruct is a test struct
    main.TestEnumInt:
      enum:
        - 1
        - 2
        - 3
      type: integer
      format: int32
    main.TestEnumString:
      enum:
        - a
        - b
        - c
        - x
        - "y"
        - z
      type: string
    time.Time:
      type: string
      format: date-time

Notice the TestEnumString was the type I was looking at here.

EDIT: So upon further investigation this works as expected when we have an enum as an object's property, but not when an enum is the top level return type. I'll see if I can find a fix for this.

@melloware melloware added question Further information is requested and removed bug Something isn't working labels Nov 7, 2023
@melloware
Copy link
Collaborator

Will close until we hear back from OP

@melloware melloware closed this as not planned Won't fix, can't repro, duplicate, stale Nov 8, 2023
@Kcazer
Copy link

Kcazer commented Nov 9, 2023

@melloware @Will-Mann-16 Thank you both for taking time to investigate the issue

I'm working on the same project as @ClementLevesque and have created a repository with a minimal reproduction. You can find it there https://github.com/Kcazer/stunning-winner

I also tried your schema with our config, but, as expected, it was working fine, but I noticed two main differences :

  • Your schema is an OpenApi3, while ours is Swagger2
  • The code generated from your schema uses the generated enum in the mock, but for us, the values are hardcoded (Object.values(GeneratedEnum) vs ['XXX', 'YYY', 'ZZZ'])

@melloware melloware reopened this Nov 9, 2023
@melloware
Copy link
Collaborator

And 6.20.0 just came out just verifying it still happens?

@Kcazer
Copy link

Kcazer commented Nov 9, 2023

Still happening with 6.20

But in the meantime, I've managed to track the main cause. It's definitely related to the use of $refs in the source schema. It seems to me than only "global" enums are imported in mock files, not the inline ones.

When using the following schema :

{
  "swagger": "2.0",
  "info": { "title": "orval-repro", "version": "1.0" },
  "definitions": {
    "y": {
      "type": "string",
      "enum": ["AAA", "BBB", "CCC"]
    }
  },
  "paths": {
    "/json": {
      "get": {
        "operationId": "JsonSample",
        "summary": "Used for issue reproduction",
        "tags": [],
        "parameters": [],
        "responses": {
          "200": {
            "description": "Simple JSON Object",
            "schema": {
              "type": "object",
              "properties": {
                "x": {
                  "type": "string",
                  "enum": ["AAA", "BBB", "CCC"]
                },
                "y": {
                  "$ref": "#/definitions/y"
                }
              }
            }
          }
        }
      }
    }
  },
  "basePath": "/",
  "consumes": ["application/json"],
  "produces": ["application/json"]
}

The generated mock will look like this

// Schema
export type JsonSample200X = (typeof JsonSample200X)[keyof typeof JsonSample200X];
export const JsonSample200X = { AAA: "AAA", BBB: "BBB", CCC: "CCC" } as const;

export type Y = (typeof Y)[keyof typeof Y];
export const Y = { AAA: "AAA", BBB: "BBB", CCC: "CCC" } as const;

export type JsonSample200 = { x?: JsonSample200X; y?: Y; };


// Mock
import { faker } from "@faker-js/faker";
import { HttpResponse, delay, http } from "msw";
import { Y } from "./generated.schemas";

export const getJsonSampleMock = () => ({
  // Inline schema => inline values, when `Object.values(JsonSample200X)` would work
  x: faker.helpers.arrayElement([
    faker.helpers.arrayElement(["AAA", "BBB", "CCC"]),
    undefined,
  ]),
  // Reference => is imported, everything works fine
  y: faker.helpers.arrayElement([
    faker.helpers.arrayElement(Object.values(Y)),
    undefined,
  ]),
});

@melloware
Copy link
Collaborator

@Kcazer is that this issue: #910 ??

@Kcazer
Copy link

Kcazer commented Nov 9, 2023

I don't think it's related, we're using different names and there is no nesting on my latest code sample

@melloware
Copy link
Collaborator

Yeah but its basically pointing to a $ref his example talks about nesting but the real issue is it points to a $ref right?

@Kcazer
Copy link

Kcazer commented Nov 9, 2023

Our issue is specifically about the case when no $ref are used. When using refs, mocks are generated with the correct typings, but when inlining enums, they become string instead of being an union of values

@melloware
Copy link
Collaborator

Undertsood. PR's are welcome...

@Will-Mann-16
Copy link
Collaborator

Will-Mann-16 commented Nov 10, 2023

PR is up for approval now. You were entirely correct, it was because it wasn't referenced externally. Adding an as const did the trick here.
CleanShot 2023-11-10 at 23 20 01@2x

@melloware melloware added this to the 6.21.0 milestone Nov 10, 2023
@melloware melloware added enhancement New feature or request bug Something isn't working and removed question Further information is requested enhancement New feature or request labels Nov 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants