Skip to content

[BUG][typescript-axios] Generated JavaScript Sets are not serialized/deserialized #11746

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

Open
5 of 6 tasks
BernhardPosselt opened this issue Feb 28, 2022 · 15 comments
Open
5 of 6 tasks

Comments

@BernhardPosselt
Copy link

BernhardPosselt commented Feb 28, 2022

Bug Report Checklist

  • Have you provided a full/minimal spec to reproduce the issue?
  • Have you validated the input using an OpenAPI validator (example)?
  • Have you tested with the latest master to confirm the issue still exists?
  • Have you searched for related issues/PRs?
  • What's the actual output vs expected output?
  • [Optional] Sponsorship to speed up the bug fix or feature request (example)
Description

The following schema generates a JavaScript Set property:

{
  "Test": {
    "required": [
      "a"
    ],
    "type": "object",
    "properties": {
      "a": {
        "uniqueItems": true,
        "type": "array",
        "items": {
          "type": "string"
        }
      }
    }
  }
}

Generated object:

export interface Test {
    /**
     * 
     * @type {Set<string>}
     * @memberof Test
     */
    'a': Set<string>;
}

However when sending this request to the server, the value is not de-serialized as an array but just replaced with an empty object ("a":{}).

Similarly, when inspecting the returned data from an API, the actual values on the JavaScript object are not wrapped in a set, but return as an array.

openapi-generator version

4.5.0

Suggest a fix

Either don't generate Set and use a T[] or add conversions for sending and receiving sets

@NoUsername
Copy link

I ran into the same issue.

As a workaround, as suggested here: #8258 (comment)

adding this typemapping fixed it for me:

(gradle kotlin dsl)

typeMappings.set(mapOf(
          // workaround for missing set serialization
          "set" to "Array"
        ))

andrewwylde added a commit to andrewwylde/openapi-generator-config that referenced this issue Apr 11, 2023
when requests go over the wire, sets should become arrays.
OpenAPITools/openapi-generator#11746 for reference
andrewwylde added a commit to Kong/openapi-generator-config that referenced this issue Apr 11, 2023
* feat: serialize sets to array for payloads

when requests go over the wire, sets should become arrays.
OpenAPITools/openapi-generator#11746 for reference
---
Co-authored-by: Drew Kimberly <[email protected]>
@JulienDeBerlin
Copy link

JulienDeBerlin commented Jul 17, 2023

I'm running into the same problem. We use the typescript-axios generator from OpenAPI Generator (maven Plugin - v. 6.6.0).
For a property like this in the openapi schema:

"catalogueTitles": {
            "uniqueItems": true,
            "type": "array",
            "items": {
              "type": "string"
            }
          }

in the process of the typescript code generation, we get

   /**
     * 
     * @type {Set<string>}
     * @memberof IsikResource
     */
    'catalogueTitles': Set<string>;

when using the generated client to send the object to the backend (SpringBoot Server) it results into a json with an empty object instead of an array:
catalogueTitles: {}

As a result our springboot controller can not handle the request.
I don't know how to handle this situation. The workarround proposed here seems to be kotlin based and it doesn't helps us.

Is a fix of this bug planned?

@BernhardPosselt
Copy link
Author

@JulienDeBerlin the quick fix is to just cast your stuff to an array/set in TypeScript.

@JulienDeBerlin
Copy link

Hi @BernhardPosselt, i guess I could convert the set into an array and then write my own axios client that accepts this customed object as param. But the all idea of using openapi was for us to use the generated DTOs and clients.

@BernhardPosselt
Copy link
Author

BernhardPosselt commented Jul 17, 2023

You always want a wrapper around the generated client because the generated client will inevitably change when you upgrade the lib. The wrapper around it will allow you to change behavior in one place and not in multiple locations. So put those casts there ;)

@JulienDeBerlin
Copy link

Actually i already use a wrapper arround the client because it was the only way i found to customize the basePath. But in that case, i'm not sure how it can help me.

For instance this is one of methods in the generated client:

   /**
     * 
     * @param {SaveResourcesParams} saveResourcesParams 
     * @param {*} [options] Override http request option.
     * @throws {RequiredError}
     * @memberof TestManagementApiImplApi
     */
    public saveResource(resource: Resource, options?: AxiosRequestConfig) {
        return TestManagementApiImplApiFp(this.configuration).saveResource(resource, options).then((request) => request(this.axios, this.basePath));
    }

The expected param is of type Resource (generated based on the openapi description):

export interface Resource {

  /**
    * 
    * @type {string}
    * @memberof Resource
    */
   'name': string;
   /**
    * 
    * @type {string}
    * @memberof Resource
    */
   'description'?: string;   
    /**
    * 
    * @type {Set<string>}
    * @memberof Resource
    */
   'catalogueTitles': Set<string>;

}
So yes i could have an alternative method in my wrapper, that accepts a CustomedResource with a
'catalogueTitles': Array<string>;

but in that case I wouldn't use the generated method at all. May be I'm missing the point.

@BernhardPosselt
Copy link
Author

use

saveResource({
    // other stuff here
    catalogTitles: ['a', 'b'] as unknown as Set<string>,
})

@JulienDeBerlin
Copy link

Got it, it's all about to silence TypeScript's type-checking. It works and was a great help. Thanks.

@adrianhelvikspond
Copy link
Contributor

This PR fixes the serialization problem: #17790

@adrianhelvikspond
Copy link
Contributor

adrianhelvikspond commented Feb 5, 2024

If you want a quick and dirty(!) hack that can break with any change to the generator, you can add this after generating the client:

# HACK: Will be solved by https://github.com/OpenAPITools/openapi-generator/pull/17790/files
(
    cd "YOUR_OUTPUT_DIRECTORY"

    convert_maps_and_sets='(function convertMapsAndSetsToPlain(value: any): any {if (typeof value !== "object" || !value) {return value;} if (value instanceof Set) { return Array.from(value).map(item => convertMapsAndSetsToPlain(item)); } if (value instanceof Map) { return Object.fromEntries([...value.entries()].map(([k, v]) => [k, convertMapsAndSetsToPlain(v)])); } if (Array.isArray(value)) { return value.map(it => convertMapsAndSetsToPlain(it)); } return Object.fromEntries(Object.entries(value) .map(([k, v]) => [k, convertMapsAndSetsToPlain(v)])); })'

    for f in `find . -name '*.ts' -type f`; do
        if cat "$f" | grep -q 'convertMapsAndSets'; then
            continue
        fi
        sed -i '' -e "s/JSON.stringify(\(.*\))$/JSON.stringify(${convert_maps_and_sets}(\1))/g" "$f"
    done
)

@adrianhelvikspond
Copy link
Contributor

@macjohnny This can be closed now too. Which version will the fix land in?

@macjohnny
Copy link
Member

looking at https://github.com/OpenAPITools/openapi-generator/milestones, this should be released in april

@cauterize
Copy link

The fix (#17790) got reverted in Version 7.6.0. Can we please reopen this bug?

@macjohnny
Copy link
Member

@cauterize do you want to fix this?

@macjohnny macjohnny reopened this Sep 24, 2024
@cauterize
Copy link

cauterize commented Sep 24, 2024

@cauterize do you want to fix this?

This is sadly way beyond my skills 🙁

We are using a common workaround on the Typescript side for now.

// Workaround for typed Sets in generated service methods
// https://github.com/OpenAPITools/openapi-generator/issues/14055
// https://github.com/OpenAPITools/openapi-generator/issues/11746

export const mapSetForContracts = <T>(set: Set<T>): Set<T> => {
  return Array.from(set.values()) as unknown as Set<T>;
};

I just wanted to reopen this for transparency reasons.

achingbrain added a commit to ipfs-shipyard/js-pinning-service-http-client that referenced this issue Oct 5, 2024
Because we have `uniqueItems: true` in our service definition yaml,
fields with `type: array` are generated as `Set`s by the typescript
generator.

Serialization/deserialization of these types is [broken](OpenAPITools/openapi-generator#11746)
so where we use these APIs [we pass arrays with `@ts-expect-error` anyway](https://github.com/ipfs/helia-remote-pinning/blob/main/src/heliaRemotePinner.ts#L155).

It doesn't look possible to override the use of Set by the generator
in a way that works (`type-mappings=set=array` produces uncompilable
code) so manually change the generated code.

Also adds a note to the readme reminding people to do it.

BREAKING CHANGE: fields that were Sets such as `PinResults.results` and `PinsGetRequest.cid` are now Arrays
achingbrain added a commit to ipfs-shipyard/js-pinning-service-http-client that referenced this issue Oct 5, 2024
Because we have `uniqueItems: true` in our service definition yaml, fields with `type: array` are generated as `Set`s by the typescript generator.

Serialization/deserialization of these types is [broken](OpenAPITools/openapi-generator#11746) so where we use these APIs [we pass arrays with `@ts-expect-error` anyway](https://github.com/ipfs/helia-remote-pinning/blob/main/src/heliaRemotePinner.ts#L155).

It doesn't look possible to override the use of Set by the generator in a way that works (`type-mappings=set=array` produces uncompilable code) so manually change the generated code.

Also adds a note to the readme reminding people to do it.

BREAKING CHANGE: fields that were Sets such as `PinResults.results` and `PinsGetRequest.cid` are now Arrays
achingbrain added a commit to ipfs-shipyard/js-pinning-service-http-client that referenced this issue Oct 5, 2024
Because we have `uniqueItems: true` in our service definition yaml, fields with `type: array` are generated as `Set`s by the typescript generator.

Serialization/deserialization of these types is [broken](OpenAPITools/openapi-generator#11746) so where we use these APIs [we pass arrays with `@ts-expect-error` anyway](https://github.com/ipfs/helia-remote-pinning/blob/main/src/heliaRemotePinner.ts#L155).

It doesn't look possible to override the use of Set by the generator in a way that works (`type-mappings=set=array` produces uncompilable code) so manually change the generated code.

Also adds a note to the readme reminding people to do it.

BREAKING CHANGE: fields that were Sets such as `PinResults.results` and `PinsGetRequest.cid` are now Arrays
achingbrain added a commit to ipfs-shipyard/js-pinning-service-http-client that referenced this issue Oct 5, 2024
Because we have `uniqueItems: true` in our service definition yaml, fields with `type: array` are generated as `Set`s by the typescript generator.

Serialization/deserialization of these types is [broken](OpenAPITools/openapi-generator#11746) so where we use these APIs [we pass arrays with `@ts-expect-error` anyway](https://github.com/ipfs/helia-remote-pinning/blob/main/src/heliaRemotePinner.ts#L155).

It doesn't look possible to override the use of Set by the generator in a way that works (`type-mappings=set=array` produces uncompilable code) so manually change the generated code.

Also adds a note to the readme reminding people to do it.

BREAKING CHANGE: fields that were Sets such as `PinResults.results` and `PinsGetRequest.cid` are now Arrays
achingbrain added a commit to ipfs-shipyard/js-pinning-service-http-client that referenced this issue Oct 8, 2024
Because we have `uniqueItems: true` in our service definition yaml, fields with `type: array` are generated as `Set`s by the typescript generator.

Serialization/deserialization of these types is [broken](OpenAPITools/openapi-generator#11746) so where we use these APIs [we pass arrays with `@ts-expect-error` anyway](https://github.com/ipfs/helia-remote-pinning/blob/main/src/heliaRemotePinner.ts#L155).

It doesn't look possible to override the use of Set by the generator in a way that works (`type-mappings=set=array` produces uncompilable code) so manually change the generated code.

Also adds a note to the readme reminding people to do it.

BREAKING CHANGE: fields that were Sets such as `PinResults.results` and `PinsGetRequest.cid` are now Arrays
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants