swagger-typescript-api icon indicating copy to clipboard operation
swagger-typescript-api copied to clipboard

"circularly references itself" errors after upgrade to 13

Open bagbag opened this issue 2 years ago • 14 comments

Command: swagger-typescript-api --extract-enums --no-client --path https://raw.githubusercontent.com/europace/baufismart-kundenangaben-api/master/kundenangaben-openapi.yaml --name kundenangaben-api.model.ts

Worked with 12, but now typescript throws "circularly references itself" errors with 13.

bagbag avatar Jul 26 '23 10:07 bagbag

Same here. Upgrading from 12 to 13 introduced circular references similar to below.

The previous output:

export interface Config {
  type?:
    | 'anExampleConfig'
    | 'anotherExampleConfig';
}

export interface AnExampleConfig extends Config  {
  field?:
    | 'one'
    | 'two'
    | 'three';
  anotherField?: boolean;
}

The new, circular dependency style:

type BaseConfigConfigTypeMapping<Key, Type> = {
  type: Key;
} & Type;

interface BaseConfig {
  configType?:
    | 'anExampleConfig'
    | 'anotherExampleConfig';
}

export type Config = BaseConfig &
  (
    | BaseConfigConfigTypeMapping<'anExampleConfig', ExampleConfig>
    | BaseConfigConfigTypeMapping<'anotherExampleConfig', AnotherExampleConfig>
  );

export interface ExampleConfig extends Config  {
  field?:
    | 'one'
    | 'two'
    | 'three';
  anotherField?: boolean;
}

rkuykendall avatar Sep 20 '23 16:09 rkuykendall

I fixed this by modifying https://github.com/acacode/swagger-typescript-api/blob/master/src/code-gen-process.js#L122-L124

to be

const componentsToParse = _.sortBy(this.schemaComponentsMap.filter(
      _.compact(['schemas', this.config.extractResponses && 'responses']),
    ), (value) => {
      return value.rawTypeData.hasOwnProperty("discriminator") ? 0 : 1;
    });

This forces the schemas that have a discriminator to execute first creating the right types.

andymac4182 avatar Dec 11 '23 11:12 andymac4182

@js2me Not sure if you have an idea how to fix this properly or if this is the proper fix for this?

andymac4182 avatar Dec 11 '23 11:12 andymac4182

I am getting similar error, can get this fixed?

NerijusD avatar Dec 18 '23 12:12 NerijusD

@NerijusD Are you able to provide another test case? Similarly, can you test this fix on your codebase?

rkuykendall avatar Jan 02 '24 15:01 rkuykendall

I can confirm https://github.com/acacode/swagger-typescript-api/issues/564#issuecomment-1849846996 significantly reduced my errors from dozens to just 2. Perhaps someone here can make a PR with a test case and patch?

rkuykendall avatar Jan 02 '24 16:01 rkuykendall

can confirm that this indeed fixes the issue

Techbinator avatar Jan 10 '24 15:01 Techbinator

Let's simplify initial example:

type BaseConfigConfigTypeMapping<Key, Type> = {
  type: Key;
} & Type;

interface BaseConfig {
  type?:
    | 'a'
    | 'b';
}

export type Config = BaseConfig &
  (
    | BaseConfigConfigTypeMapping<'a', AConfig>
    | BaseConfigConfigTypeMapping<'b', BConfig>
  );

export interface AConfig extends Config  {
  ... few unique fields
}

export interface BConfig extends Config  {
  ... few unique fields
}

I assume that instead of

export interface AConfig extends Config  {
  ... few unique fields
}

export interface BConfig extends Config  {
  ... few unique fields
}

lib should generate next lines and use BaseConfig instead of Config

export interface AConfig extends BaseConfig {
  ... few unique fields
}

export interface BConfig extends BaseConfig {
  ... few unique fields
}

IhnatKlimchuk avatar Feb 06 '24 15:02 IhnatKlimchuk

Hi!

I am facing the same issue. Does anyone have any information on this?

Thanks in advance

pablo-olmedo-poex avatar May 16 '24 13:05 pablo-olmedo-poex

@pablo-olmedo-poex Does this fix your issue? https://github.com/acacode/swagger-typescript-api/issues/564#issuecomment-1849846996

If not, can you provide some examples?

rkuykendall avatar May 17 '24 01:05 rkuykendall

@pablo-olmedo-poex Does this fix your issue? #564 (comment)

If not, can you provide some examples?

This fix just reduced the amount of errors from 10 to 4 😢

pablo-olmedo-poex avatar May 17 '24 06:05 pablo-olmedo-poex

@pablo-olmedo-poex Does this fix your issue? #564 (comment)

If not, can you provide some examples?

Hey, would like to add i also have this issue, i've tried the latest versions of mayor version 11-13. The error suddenly appearing is probably because of a change of how the type is defined in the API.

This is the command used to generate: npx swagger-typescript-api -p <The API path> -o ./libs/search/src/lib/apiClient --modular. I also tried the fix mentioned in #564, by just editing the code in node_modules, did not work.

This is what it used to generate (the relevant parts):

type UtilRequiredKeys<T, K extends keyof T> = Omit<T, K> & Required<Pick<T, K>>;

export type Geometry = (
	| UtilRequiredKeys<Point, 'type'>
	| UtilRequiredKeys<LineString, 'type'>
	| UtilRequiredKeys<Polygon, 'type'>
	| UtilRequiredKeys<MultiPoint, 'type'>
	| UtilRequiredKeys<MultiLineString, 'type'>
	| UtilRequiredKeys<MultiPolygon, 'type'>
) & {
	type: string;
};

export interface LineString {
	/** LineString */
	type: string;
	/** Coordinates in WGS 84 format */
	coordinates: number[][];
}

// Including LineString is: Point, Polygon, MultiPoint etc

This is what its generating now with typescript errors in the comments:

type UtilRequiredKeys<T, K extends keyof T> = Omit<T, K> & Required<Pick<T, K>>;

// Type alias 'Geometry' circularly references itself.ts(2456)
export type Geometry = (Point | LineString | Polygon | MultiPoint | MultiLineString | MultiPolygon) & {
	type: string;
};

// Type alias 'LineString' circularly references itself.ts(2456)
export type LineString = UtilRequiredKeys<Geometry, 'type'> & {
	/** LineString */
	type: string;
	/** Coordinates in WGS 84 format */
	coordinates: number[][];
};

// Including LineString is: Point, Polygon, MultiPoint etc

jonaskris avatar May 22 '24 13:05 jonaskris

This fix just reduced the amount of errors from 10 to 4 😢

It seems like implementing this fix in a PR is something anyone in this thread could do to get us 60% closer to a solution. I know it significantly reduced (but did not eliminate) errors in my use-case as well.

rkuykendall avatar May 23 '24 13:05 rkuykendall