useDateType should convert datetime strings to date objects.

[seriouslag]

Describe the problem

useDateType marks the models as Date objects. The service does not convert the date strings to date objects.

This is causing the model generated as (Date) to be incorrect compared to the actual data (string). This library should take care of the conversion.

Describe the proposed solution

The useDateType should convert the string in the datetime format to a JS Date object so the data types match.

Alternatives considered

No response

Importance

nice to have

Additional information

No response

[mrlubos]

Hey @seriouslag, this will never be a part of the client package. Client deals with sending requests, nothing more. However, we will support middleware, which would unlock this feature

[seriouslag]

Hey @seriouslag, this will never be a part of the client package. Client deals with sending requests, nothing more. However, we will support middleware, which would unlock this feature

Why add a property that breaks the contract of what is generated?

[mrlubos]

Here's context for that feature https://github.com/ferdikoomen/openapi-typescript-codegen/pull/494

[AnderssonPeter]

Can't the middleware also handle the type change?

[mrlubos]

@AnderssonPeter no. Types are generated during the build, the actual conversion needs to happen during runtime

[AnderssonPeter]

@AnderssonPeter no. Types are generated during the build, the actual conversion needs to happen during runtime

So for each new type I would need to modify the generator and add a new flag?

[mrlubos]

@AnderssonPeter can you explain what you mean by adding a new flag?

[AnderssonPeter]

@AnderssonPeter can you explain what you mean by adding a new flag?

The useDateType flag, but i guess adding new types might not be that common, so it's maybe a none issue.

But the current implementation of useDateType does more harm than good, as the typescript definition says it will be a Date but it's still a string...

[mrlubos]

@AnderssonPeter that's why it's disabled by default. Please see usage in https://github.com/ferdikoomen/openapi-typescript-codegen/pull/494

[seriouslag]

@AnderssonPeter that's why it's disabled by default. Please see usage in ferdikoomen/openapi-typescript-codegen#494

I understand the need for this.

My suggestion would be to register middleware on build to overwrite the needed types and have the same middleware registered on runtime to do the conversions.

interface Property {
  type: string;
  format?: string;
  // other common properties to do conversions
  // can expand later
}

interface Middleware<UncovertedType, ConvertedType> {
  /**
    * path(s) to custom class/type to import for generated code types to work;
    * Could be left empty in the case of Date
    */
   import?: string|string[]|((property: Property) => (string|string[]));
   /**
    * This is the string representation of what will be set for the type in the generated files
    * Would be 'Date' for the case of Date
    */
   convertedType: string;
   /**
    * This will be used on build and runtime
    * Checks if the property should be converted
    * checkProperty: (prop) => prop.type === 'string' && prop.format === 'date-time',
    */
  checkProperty: (property: Property) => boolean
  /**
    * This will be used on runtime clients to convert data at the service layer.
    * property is the OpenAPI definition ;
    * 
    * Convert data would would only be called if checkProperty is true.
    * convertData: (data, _prop) => new Date(data),
    */
  convertData: (data: UncovertedType, property: Property) => ConvertedType;
}

const DateMiddleware: Middleware<string, Date> = {
  convertedType: 'Date',
  checkProperty: (prop) => prop.type === 'string' && prop.format === 'date-time',
  convertData: (data, _prop) => new Date(data),
}

Something like this approach would allow library consumers to extend the type-overriding system and offer a standard middleware API.

[mrlubos]

@seriouslag yeah, something like that if we pass the original definition/schema to the runtime. Otherwise you have to loop through fields and check if string is convertible to date, which might not be a bulletproof approach

[reneweteling]

Hey guys, so if i understand this correctly by adding the intercepted middleware, the output will be changed, only the type def will be the same, so the output would be

createdAt: new Date('2024-01-01'):string this isn't the way to go isn't it?

isn't it possible to register a interceptor or middleware in the config when you create the client from the OAS spec?

Also what is the propper way of installing this middleware?

I've tried this but that's obviously not working

OpenAPI.interceptors.response.use(async (response) => {
  await DateMiddleware(response); // async
  return response; // <-- must return response
});

Sorry, I'm a Ruby, Elxir dev, don't touch TS that much.

[reneweteling]

Ok, so now im getting the correct date types in my models, only the output is still a string, guessing that's why we need the interceptor, only how do you hookt it all together?

openapi-ts -i ../server/swagger/swagger.yaml -o ./src/api/client --client fetch --exportSchemas true --name Client --useDateType true

This works :D

export type Token = {
  /**
   * Bearer token
   */
  accessToken: string;
  /**
   * Refresh token
   */
  refreshToken: string;
  /**
   * Token expiration time in seconds
   */
  expiresIn: number;
  /**
   * Creation time
   */
  createdAt: Date;
  /**
   * Token type
   */
  tokenType: string;
};

output of createdAt is still a string through.

[mrlubos]

That's right @reneweteling. What does your DateMiddleware look like? How do you know which endpoint + fields to transform?

[reneweteling]

@mrlubos well, just to be clear, i have no clue :D

Current state of affairs, when I add the --useDataType true the types get correctly generated, only the response is still a string. So I would like to return a Date object instead of a string.

Then I tried to create a interceptor like so

// DOES NOT WORK
OpenAPI.interceptors.response.use(async (response) => {
  console.log(response);
  return response; // <-- must return response
});

// client that is not authenticated
export const OpenClient = new Client({ ...OpenAPI, TOKEN: undefined });

// WORKS :D
OpenClient.request.config.interceptors.response.use(async (response) => {
  console.log(response);
  return response;
});

Issue here is, that its just as you say, you only get the response, not the correlating schema definition. So trying to hook up @seriouslag's suggestion has not worked yet.

Do you have an idea on how to get the correct schema in the response interceptor? Or do you know how I can transform the dates properly?

Thanks!!

[seriouslag]

@mrlubos well, just to be clear, i have no clue :D

Current state of affairs, when I add the --useDataType true the types get correctly generated, only the response is still a string. So I would like to return a Date object instead of a string.

Then I tried to create a interceptor like so

// DOES NOT WORK

OpenAPI.interceptors.response.use(async (response) => {

  console.log(response);

  return response; // <-- must return response

});



// client that is not authenticated

export const OpenClient = new Client({ ...OpenAPI, TOKEN: undefined });



// WORKS :D

OpenClient.request.config.interceptors.response.use(async (response) => {

  console.log(response);

  return response;

});

Issue here is, that its just as you say, you only get the response, not the correlating schema definition. So trying to hook up @seriouslag's suggestion has not worked yet.

Do you have an idea on how to get the correct schema in the response interceptor? Or do you know how I can transform the dates properly?

Thanks!!

Hey there, my suggestion was for inspiration to bring change to this library as the problem you are facing is the same others are facing. The current implementation of --useDateType causes confusion and generates broken code by default.

[reneweteling]

@seriouslag yep, getting there :D thanks though, and sorry for the confusion

[mrlubos]

Just a quick update, we will be addressing this after client packages are done https://github.com/hey-api/openapi-ts/issues/308

[jordanshatford]

NOTE, this config option will be changed:

export default {
  input: 'path/to/openapi.json',
  output: 'src/client',
  type: {
    dates: true,
  },
}

ref: https://heyapi.vercel.app/openapi-ts/migrating.html#removed-usedatetype

[Nick-Lucas]

Why can't transformation code be generated to "new Date()" at each site?

That would be far more valuable than changing the types and still having no solution to understand what values should become dates at runtime. It would be a killer feature for a package like this

[jordanshatford]

@Nick-Lucas that may be possible. I would need to look more into it, one issues is that I dont think each endpoint has that information right now. This would be a feature to add in

[Nick-Lucas]

Was just having a scan and it looks like the generator should have access to what's needed as the Model type is quite rich: https://github.com/hey-api/openapi-ts/blob/840d9df6aaa58025b8a364419fffdc7937575901/packages/openapi-ts/src/openApi/common/interfaces/client.ts#L87-L116

So hopefully "just" a case of iterating over the properties of the response during codegen and generating a transform for any date/date-time typed schemas in the response model

I don't have a ton of time this week but might at some stage be able to help contribute this if needed

[jordanshatford]

@mrlubos well, just to be clear, i have no clue :D

Current state of affairs, when I add the --useDataType true the types get correctly generated, only the response is still a string. So I would like to return a Date object instead of a string.

Then I tried to create a interceptor like so

// DOES NOT WORK

OpenAPI.interceptors.response.use(async (response) => {

  console.log(response);

  return response; // <-- must return response

});



// client that is not authenticated

export const OpenClient = new Client({ ...OpenAPI, TOKEN: undefined });



// WORKS :D

OpenClient.request.config.interceptors.response.use(async (response) => {

  console.log(response);

  return response;

});

Issue here is, that its just as you say, you only get the response, not the correlating schema definition. So trying to hook up @seriouslag's suggestion has not worked yet.

Do you have an idea on how to get the correct schema in the response interceptor? Or do you know how I can transform the dates properly?

Thanks!!

This issue with interceptors has been fixed. You can now pass an OpenAPI object with interceptors to the client and they will be used.

[mrlubos]

@Nick-Lucas I think what you're saying sounds about right and how we're thinking about approaching it. One distinction, Model is not available after generating the client. Instead, we could get this information from schemas.gen.ts, that's how we might solve this

[Nick-Lucas]

Interesting, I was assuming the most performant way at runtime would be to generate a bunch of code like

class SomeService {
  async someMethod() {
    body = await fetch()
    
    # generate safe coersion for each date property
    if (body?.path?.to?.date) {
      body.path.to.date = new Date(body.path.to.date)
    }

    # also generate loops over any inner arrays etc

    return result
  }
}

Reading the schemas at runtime I guess wouldn't be crazy expensive but hard to beat this when you're doing codegen and know all the data structures up-front right? Or would it negatively affect bundle size for frontend clients?

[mrlubos]

@Nick-Lucas that's the trade-off to consider. Ideally we could measure both approaches and see which works better for us

[caesay]

We are looking for type safety by using a generator, which means the "conversion from string to Date" needs to be done per property at code-gen time in my opinion. The idea of using an interceptor and changing anything that looks like a date, into a Date, is just really really bad. The generator knows that the source type is a string but the intended type is a Date so why do we need to guess at runtime?

Imagine the following config:

export default defineConfig({
  types: {
    "date-time": {
      typeName: "Date",
      typeConverter: "new Date($1)"
    } 
  }

This kind of model would allow far more flexibility and safety than a middleware. It unlocks new ways of date parsing:

export default defineConfig({
  types: {
    "date-time": {
      typeName: "moment",
      typeConverter: "moment($1)"
    } 
  }

Or even the ability to override arbitrary types to create a custom parser for a specific object type. Beyond the syntax above the only other thing I can see that's missing is the ability to specify where those types and/or helper functions come from so they can be imported.

[mrlubos]

@caesay see my note https://github.com/hey-api/openapi-ts/issues/181#issuecomment-2099186192. Depending on the spec and payload size, this could really bloat your bundle

[caesay]

@caesay see my note #181 (comment). Depending on the spec and payload size, this could really bloat your bundle

My suggestion won't hardly change the bundle size at all. If I understand your comment, you're suggesting the api schema is passed to interceptors (which I agree, would bloat your bundle). I'm not suggesting that. I'm suggesting that at code gen time we allow types to be converted using custom methods using the syntax I provided above. My suggestion does not require interceptors at all since the conversion is written into the generated api services.

[mrlubos]

I am not sure I am following @caesay. Can you provide a sample service output with your approach? Don't worry about accuracy, just looking for the idea. How would it then know which field to transform?