mrjono1 / joi-to-typescript

Convert Joi Schemas to TypeScript interfaces
MIT License
125 stars 39 forks source link

Extend interfaces when extending schemas #53

Open ThomasKuhlmann opened 3 years ago

ThomasKuhlmann commented 3 years ago

Hi!

I just realised that if you extend a schema, e.g.

const WorkspaceSchema = Joi.object({
 statistics:Joi.object({
     views: Joi.number()
     })
}).label('IWorkspace')

const ProjectSchema = WorkspaceSchema.keys({
  statistics:Joi.object({
    users:Joi.number()
    })
}).label('IProject')

You end up with two interfaces:

export interface IWorkspace {
  statistics: {
    views: number;
  };
}

and

export interface IProject {
  statistics: {
    users: number
   }
}

Wouldn't it make more sense to extend IProject with IWorkspace?

export interface IProject extends IWorkspace {
  statistics: {
    users: number
   }
}
mrjono1 commented 3 years ago

That would be nicer, I'm not sure if Joi exposes this type of information, but this can be investigated

ThomasKuhlmann commented 3 years ago

I had a look around as well, but the documentation wasn't entirely conclusive... I've posted a question to the Joi repository, let's see what they come back with

https://github.com/sideway/joi/issues/2549

ThomasKuhlmann commented 3 years ago

@mrjono1 The team at Joi said the information isn't exposed by default. They had another suggestion to use .note() instead of .describe()?

https://github.com/sideway/joi/issues/2549#issuecomment-772010508

Not sure if that helps? I just realised that this isn't just a cosmetic issue, it seems part of the validation rules get lost when nested schemas get merged. If I extend this:

export const WorkspaceSchema = Joi.object({
  statistics: Joi.object({
    views: Joi.number().default(0),
    members: Joi.number().default(0),
  }).default(),
}).default()
  .label("IWorkspace")

with this:

export const PortfolioSchema = WorkspaceSchema.keys({
  statistics: Joi.object({
    risks: Joi.object({
      open: Joi.number().default(0),
      closed: Joi.number().default(0),
    }).default(),
  }).default(),
}).default()
  .label("IPortfolio")

you end up with these two interfaces:

IWorkspace

export interface IWorkspace {
  statistics: {
    members: number;
    views: number;
  };
}

IPortfolio

export interface IPortfolio {
  statistics: {
    risks: {
      closed: number;
      open: number;
    };
  };
}

But that's incorrect. Even if it doesn't end up extending ( interface IPortfolio extends IWorkspace), it should still merge the two statistics fields in the IPortfolio interface, shouldn't it?

export interface IPortfolio {
  statistics: {
    members: number;
    views: number;
    risks: {
      closed: number;
      open: number;
    };
  };
}
mrjono1 commented 3 years ago

It looks like they mean .label() vs .note(), as .describe() is what exports the Joi into a its own style of json schema. (I don't mean to be rude to the other team this isn't their library) Step 1 Supporting .label() and .note() to be the interface name sounds straight forward. There should only be one bit of code that determines the interface name eg if label does not exist use note

Step 2 Figuring out if you can do this, it does sound complicated but possible

One concern though is people may be using .note() the same way as .description() but I guess I could just note that as a FAQ item. I currently ignore note

mrjono1 commented 3 years ago

I also found Joi.id() and Joi.extract() they seem interesting FYI I'm using .label() as it allows your joi to also work with https://github.com/glennjones/hapi-swagger

kingmesal commented 2 years ago

This would be a VERY helpful feature to have. If it is not able to be discovered via the Joi api, a solution using label or note seem very acceptable, if implemented in such a way that when specified it will add the extends NAME to the created interface.

Happy to try to help, not sure where to start in this codebase to create the functionality though.

mrjono1 commented 2 years ago

In the code where it gets the interface name, i think it will have an array of 'className's so you should be able to tell what to extend, but figuring out which properties are on the base and current will be hard

kingmesal commented 2 years ago

I have found in Joi where the data is, and modifying a test to be able to cover an implementation will be easy, because you already test concat. However, I cannot figure out where you write out the actual export interface Foo

    "metas": [
      {
        "className": "Key"
      },
      {
        "className": "Metadata"
      },
      {
        "className": "Value"
      },
      {
        "className": "CombinedRecord"
      }
    ],

it should just be able to write out export interface CombinedRecord extends Value, Metadata, Key

All the other logic is already in place for having properly created the interface. I'm just trudging through the code and have a rather difficult time interpreting where that work is being done.

kingmesal commented 2 years ago

Can you point me to something that might help me better understand how to modify the code base to add the "extends" to the output template?

mrjono1 commented 2 years ago

yes the code has gotten a bit confusing, I'm not sure where we would need to add this concept, it will take time to figure out

mrjono1 commented 2 years ago

I've made a start this adds the extends part https://github.com/mrjono1/joi-to-typescript/pull/235 next is to remove the fields that are being extended this may take a bit to tidy things up to make this do able