[BUG] [typescript-fetch] Request Parameter Interfaces Conflict with remove-operation-id-prefix

[bradenmacdonald]

Bug Report Checklist

• Have you provided a full/minimal spec to reproduce the issue?
• Have you validated the input using an OpenAPI validator (example)?
• What's the version of OpenAPI Generator used?
• Have you search for related issues/PRs?
• What's the actual output vs expected output?
• [Optional] Bounty to sponsor the fix (example)

Description

When an API spec contains two similarly named operations that differ only in prefix:

      operationId: tag_new_create
...
      operationId: unit_new_create

then generating the typescript-fetch client with the --remove-operation-id-prefix option will result in an error like:

apis/index.ts:2:1 - error TS2308: Module './TagApi' has already exported a member named 'NewCreateRequest'. Consider explicitly re-exporting to resolve the ambiguity.

openapi-generator version

This is in 4.0.0beta. The same error did not occur with the old 3.x typescript-fetch, though this new implementation is generally much better.

OpenAPI declaration file content or url

https://gist.github.com/bradenmacdonald/061c37b42af22832a14574e4aeaa5c59

Command line used for generation

java -jar openapi-generator-cli-4.0.0-beta.jar generate \
    --input-spec api-spec.yaml \
    --generator-name typescript-fetch \
    --output api_client \
    --config openapi-generator-config.json \
    --remove-operation-id-prefix

Steps to reproduce

Simply generate the typescript-fetch client using the sample gist and the command above, then run cd api_client; npm install; npm run build

Related issues/PRs

I couldn't find any existing reports of this issue.

Suggest a fix

Ideally just never omit the prefixes when generating the request parameter interfaces, so that the interface is still called e.g. TagNewCreateRequest (instead of interface NewCreateRequest) regardless of the --remove-operation-id-prefix option (the main reason I use that option is for nicer method names; the names of the parameter interfaces aren't often used directly so it's fine if they're more verbose).

Alternately, put each module's interfaces into a namespace for that module, so that one must use e.g. TagApi.NewCreateRequest to get the interface after importing the overall API.

[auto-labeler]

👍 Thanks for opening this issue!
🏷 I have applied any labels matching special text in your issue.

The team will review the labels and make any necessary changes.

[janbuchar]

This conflict also happens when I already have a model definition that is accidentally named {operaitionId}Request

[jgiles]

^ I was just constructing a repro case to file a fresh issue for what @Teyras mentioned above. We have some model definitions named Request by design, and that makes the typescript-fetch generation unusable for us (we're trying to move off of swagger-codegen and this blocks us).

[jgiles]

Repro case:

example_spec.yaml

swagger: "2.0"
info:
  title: "Example Clash"
  version: "1.0"
schemes:
- https
host: http://localhost
consumes:
- application/json
produces:
- application/json
paths:
  /v1/foos:
    post:
      operationId: CreateFoo
      responses:
        "200":
          description: A successful response.
          schema:
            $ref: '#/definitions/Foo'
      parameters:
      - name: body
        in: body
        required: true
        schema:
          $ref: '#/definitions/CreateFooRequest'
      tags:
      - Example
definitions:
  CreateFooRequest:
    type: object
    properties:
      nickname:
        type: string
    required:
    - nickname
  Foo:
    type: object
    properties:
      id:
        type: string
      nickname:
        type: string
    required:
    - id
    - nickname

Snippet from generated src/api.ts:

import {
    CreateFooRequest,
    CreateFooRequestFromJSON,
    CreateFooRequestToJSON,
    Foo,
    FooFromJSON,
    FooToJSON,
} from '../models';

export interface CreateFooRequest {
    body: CreateFooRequest;
}

/**
 * no description
 */
export class ExampleApi extends runtime.BaseAPI {

    /**
     */
    async createFooRaw(requestParameters: CreateFooRequest): Promise<runtime.ApiResponse<Foo>> {
        if (requestParameters.body === null || requestParameters.body === undefined) {
            throw new runtime.RequiredError('body','Required parameter requestParameters.body was null or undefined when calling createFoo.');
        }

        const queryParameters: runtime.HTTPQuery = {};

        const headerParameters: runtime.HTTPHeaders = {};

        headerParameters['Content-Type'] = 'application/json';

        const response = await this.request({
            path: `/v1/foos`,
            method: 'POST',
            headers: headerParameters,
            query: queryParameters,
            body: CreateFooRequestToJSON(requestParameters.body),
        });

        return new runtime.JSONApiResponse(response, (jsonValue) => FooFromJSON(jsonValue));
    }

   /**
    */
    async createFoo(requestParameters: CreateFooRequest): Promise<Foo> {
        const response = await this.createFooRaw(requestParameters);
        return await response.value();
    }

}

(see that we both import and declare the type CreateFooRequest)

The generated code does not compile:

$ tsc
src/apis/ExampleApi.ts:37:31 - error TS2339: Property 'body' does not exist on type 'CreateFooRequest'.

37         if (requestParameters.body === null || requestParameters.body === undefined) {
                                 ~~~~

src/apis/ExampleApi.ts:37:66 - error TS2339: Property 'body' does not exist on type 'CreateFooRequest'.

37         if (requestParameters.body === null || requestParameters.body === undefined) {
                                                                    ~~~~

src/apis/ExampleApi.ts:52:60 - error TS2339: Property 'body' does not exist on type 'CreateFooRequest'.

52             body: CreateFooRequestToJSON(requestParameters.body),
                                                              ~~~~

src/index.ts:3:1 - error TS2308: Module './apis' has already exported a member named 'CreateFooRequest'. Consider explicitly re-exporting to resolve the ambiguity.

3 export * from './models';
  ~~~~~~~~~~~~~~~~~~~~~~~~~


Found 4 errors.

[jgiles]

#3695 is the start of a fix along the lines of the namespacing suggestion from @bradenmacdonald (which fixes both of the naming collision issues discussed in this thread).

Note that adding namespacing can be a breaking change for clients of the generated code. So that this does not have to wait for a major release (based on policies in https://github.com/OpenAPITools/openapi-generator/wiki/Git-Branches ), I think I should add an option to control this behavior?

[jgiles]

Updated: change the namespacing flag to prefixing flag.

Starting with release 4.1.1 (which should be out soon), you can avoid this issue by passing

--additional-properties=prefixParameterInterfaces=true

on the command line with your generate command.

Or, adding "prefixParameterInterfaces": true to myconfig.json and passing -c myconfig.json with your generate command

[jgiles]

So... @macjohnny @bradenmacdonald I discovered while trying to incorporate the new generated code into my team's build today that Typescript namespaces are not well-supported by Babel (often used in React projects like ours). I think the specific usage of namespaces added here actually would work with Babel after a slight tweak to add declare to the namespace declaration (according to https://babeljs.io/docs/en/babel-plugin-transform-typescript). But, the isolatedModules compiler option often added to .tsconfig files as a way to ensure Babel compatibility forbids all namespace usage (you get errors like Cannot compile namespaces when the '--isolatedModules' flag is provided. upon running tsc).

All of this could probably be worked around with some creative tooling setup, but I wonder if it would be better to abandon namespaces entirely and instead do ugly but tooling-friendly string prefixing/suffixing (e.g. PetApiAddPetRequest). This would "just work" with more tooling setups. The ugly names might not be so bad, because most of the time clients can just pass anonymous object literals, and otherwise they could alias the type.

I bring this up now because if we don't want to use namespaces here, it would be good not to release the current (new) behavior in 4.1.1.

[macjohnny]

@jgiles thabks for the analysis, go ahead. It should be as easy as removing the dot, right?

[jgiles]

Yeah it's simpler actually than the namespacing was. How close is the 4.1.1 release? I can try to make the change tomorrow.