TCMiranda / joi-extract-type

Provides native type extraction from Joi schemas for Typescript
MIT License
152 stars 27 forks source link

Joi 16? #22

Open a3s opened 4 years ago

a3s commented 4 years ago

Does this library have support (or upcoming support) for Joi 16?

TCMiranda commented 4 years ago

@a3s yes, but I don't thing anyone had the chance to look into it yet.

I think the first step would be to test the current implementation against the new version, and document what needs to be done in order to support it.


Changelog: https://github.com/hapijs/joi/issues/2037

guyellis commented 4 years ago

A few of us just spent an hour mob hacking on trying to get this to support 16. Running into some issues. Joi 16 exports an interface:

declare namespace Joi {
...

whereas Joi 15 exported everything from the root.

We wrapped extractType etc. in the same namespace but struggling to work out how to merge those namespaces.

https://www.typescriptlang.org/docs/handbook/declaration-merging.html#merging-namespaces

Ideas?

guyellis commented 4 years ago

@TCMiranda - feel free to assign this issue to me for now. If I'm unable to do a PR I'll let you know.

TCMiranda commented 4 years ago

Hi @guyellis, seems that we cannot augment non-exported interfaces within namespaces (which is the third example on the link you shared) right?

We need to access (at least) the schema's interfaces to make them generic and infer the type. The easiest fix would be to export them from @types/hapi__joi

guyellis commented 4 years ago

@TCMiranda - are you suggesting a PR on @types/hapi__joi to add those or something like an import and then export in another module or something else?

TCMiranda commented 4 years ago

I think that import and then export in another module wont work. Because we need to augment the original interface. My guess is that if we export the interface inside the namespace used on @types/hapi__joi it will make it augmentable.

Problem is that I tried to simulate it and it seems that even when the interface is exported it still can't be merged.

I need to explore some more time to understand if it is possible.

The other option is to "rewrite" @types/hapi__joi completely without namespaces. There is a similar library that uses this approach

TCMiranda commented 4 years ago

@guyellis I opened a branch https://github.com/TCMiranda/joi-extract-type/tree/feature/joi16 to work on it. I managed to make some stuff work. To use it you need to manually patch @types/hapi__joi declaration file like so: Screenshot from 2019-10-18 20-51-26

guyellis commented 4 years ago

@TCMiranda - I don't see any commits in the joi16 branch beyond Sep 19 (a month ago) - did you push them to GitHub?

TCMiranda commented 4 years ago

Sorry! Here it is https://github.com/TCMiranda/joi-extract-type/tree/feature/joi16

guyellis commented 4 years ago

Hi @TCMiranda - I sent you an email a couple of days ago asking if you were available to pair on this on a Zoom session - not sure if you got the email - I sent it to the email that's listed here on your GitHub account.

I'm having a tough time trying to step through and understand how TypeScript is deriving the types. I'm stuck at exactly the same point that you commented on in the code in this branch where you can't work out why every schema is accepted by T extends StringSchema<infer O>. I'm thinking that some rubber-duck debugging with you, me and (if available) a couple of my coworkers might get us to the solution. Let me know what you think.

guyellis commented 4 years ago

@TCMiranda with help from @chriscrewdson @kschat @jasonoverby @llwt @iybaron we came up with this example snippet to answer the "debug" question you have in your branch.

// debug, trying to understand why every schema is accepted within this extends clause

interface One {
  foo: string;
}

interface Two {
  foo: string;
}

type Three<T> =  T extends Two ? 'yes' : 'no';

const a: Three<One> = 'yes'; // compiles okay
const b: Three<Two> = 'yes'; // compiles okay
const c: Three<One> = 'no'; // Type '"no"' is not assignable to type '"yes"'.ts(2322)

TypeScript is not distinguishing between differently named interfaces that have identical shapes.

If you add, for example, a dummy __stringSchema(): void; prop in the StringSchema<> interface and do something similar like __numberSchema(): void; in the NumberSchema<> interface it will get you past that problem.

So in my example:


interface One {
  foo: string;
  __one: string;
}

interface Two {
  foo: string;
  __two: string;
}

type Three<T> =  T extends Two ? 'yes' : 'no';

const a: Three<One> = 'yes'; // Type '"yes"' is not assignable to type '"no"'.ts(2322)
const b: Three<Two> = 'yes'; // compiles okay
const c: Three<One> = 'no'; // compiles okay
guyellis commented 4 years ago

A coworker @llwt found the reason why they wrapped the DefinitelyType definitions in a namespace: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/38361#issuecomment-531754321

TCMiranda commented 4 years ago

Hey @guyellis sorry, I'm back! So, I saw the comment on DefinitelyTyped. I didn't fully understand the problem that they had, but it's ok, I think we are close to solve this if they accept to export the interfaces within the namespace like I suggested here: https://github.com/TCMiranda/joi-extract-type/issues/22#issuecomment-544012840

It was awesome that you found the reason of the "extends problem", thanks! So with the interfaces exported, we could augment them with those dummy props like you said:

a dummy __stringSchema(): void; prop

Right?


Also to remember, although we are close to "solve" technical impediments, we still need to adapt the API to the changes on joi@16: hapijs/joi#2037

guyellis commented 4 years ago

@TCMiranda - hope you had a good break/vacation wherever you went.

Yes - I think that a dummy property will allow tsc to disambiguate between two identically shaped interfaces and address this problem.

Yes, once the technical problems are solved we still need to adapt the new API. I feel that this can be done in two phases:

The only reason I suggest the two phases is so that we don't delay a usable v16 of joi-extract-type. My guess is that new features won't be used right away for people upgrading to 16 so these can be incrementally added. However, if it's super easy and quick to add this then obviously we can do it all at once.

TCMiranda commented 4 years ago

Ok, we can also release it as a tag after the first step, so that a normal installation would be resolved to Joi@15, but it would also be possible to install the @16 version for the ones that need it even while on "wip".

I guess the first step is to make a PR to DefinitelyTyped to export the interfaces, do you agree?

guyellis commented 4 years ago

I guess the first step is to make a PR to DefinitelyTyped to export the interfaces, do you agree?

Yes, I agree.

@RecuencoJones - do you see any problems with a PR to DefinitelyTyped to export the interfaces for Joi@16? TL;DR - this repo, joi-extract-type, augments the Joi types to provide TypeScript types for Joi Schema. In order to augment those types they need to be exported like they were in Joi@15.

guyellis commented 4 years ago

Also @SimonSchick @Marsup @jaulz what are your thoughts on the previous comment? Mentioning you because you were involved in the PR for Joi@16 for DefinitelyTyped https://github.com/DefinitelyTyped/DefinitelyTyped/pull/38361

RecuencoJones commented 4 years ago

Hi @guyellis, I'm not really sure how you expect these interfaces to be exported, aren't they accessible from the Joi namespace already?

I.e,

import Joi from '@hapy/joi'

const foo: Joi.AnySchema
guyellis commented 4 years ago

@RecuencoJones - yes they are accessible from the Joi namespace and anyone using those typings in their code gets the benefits of them. Thank you for working on this and doing this for the benefit of the community!

The issue here is that we are trying to augment and "extend" that typing definition file. joi-extract-type allows users to generate TypeScript types from their Joi Schemas which allows them to DRY their code and write their schema once and also get a type from it. Does that make sense?

So it seems that unexported interfaces inside a namespace cannot be augmented.

@TCMiranda - did I summarize that accurately?

TCMiranda commented 4 years ago

Yes @guyellis you did. @RecuencoJones I sent an example here: https://github.com/TCMiranda/joi-extract-type/issues/22#issuecomment-544012840

RecuencoJones commented 4 years ago

I see, if I recall correctly there's a problem with exporting multiple variables/interfaces in the root when there's export = syntax.

Let me check if it would be possible to somehow have both of them, maybe using export default instead of export = could work so all exports coexist as root declarations.

RecuencoJones commented 4 years ago

Let me check if it would be possible to somehow have both of them, maybe using export default instead of export = could work so all exports coexist as root declarations.

~@guyellis @TCMiranda looks like this is possible, please let me know if this would work for you: https://github.com/RecuencoJones/DefinitelyTyped/blob/feature/export-types-and-interfaces-in-root/types/hapi__joi/index.d.ts~

Edit: nvm, this would break Joi imports, as they have to follow the syntax import * as Joi from '@hapi/joi';

guyellis commented 4 years ago

What a pity @RecuencoJones - I was just typing up a response that I thought that it would work - thanks for trying that!

TCMiranda commented 4 years ago

Hi @RecuencoJones

I see, if I recall correctly there's a problem with exporting multiple variables/interfaces in the root when there's export = syntax.

We need interfaces exported inside the namespace. I think that the problem you refer is for top level exports, right? Is it stills a problem?

Eg.

declare namespace Joi {
    export interface AnySchema extends JoiObject { }
    export interface BooleanSchema extends AnySchema { }
}
RecuencoJones commented 4 years ago

We need interfaces exported inside the namespace. I think that the problem you refer is for top level exports, right? Is it stills a problem?

Eg.

declare namespace Joi {
    export interface AnySchema extends JoiObject { }
    export interface BooleanSchema extends AnySchema { }
}

Ah, no, if that's the case that should be absolutely possible and fine 😄

RecuencoJones commented 4 years ago

Had to disable the rule strict-export-declare-modifiers but otherwise looks fine:

https://github.com/RecuencoJones/DefinitelyTyped/blob/export-types-and-interfaces-from-joi-namespace/types/hapi__joi/index.d.ts

SimonSchick commented 4 years ago

Also I should mention that eran is trying to move typings into the hapi ecosystem so you might want to have a chat with him in regards to this.

TCMiranda commented 4 years ago

@RecuencoJones thank you, that's awesome! :tada:

@guyellis now we can move on to:

something similar like __numberSchema(): void; in the NumberSchema<> interface it will get you past that problem.

guyellis commented 4 years ago

Also I should mention that eran is trying to move typings into the hapi ecosystem so you might want to have a chat with him in regards to this.

In theory, if the typings file remains the same (the modified one with the exports), then it should not matter if it's pulled in from DefinitelyTyped or if it's bundled with the Joi module. My preference is to have the typings files bundled with the module as Eran is suggesting as it's easier to keep the typings insync with the code.

Marsup commented 4 years ago

We're also willing to consider changes to joi if typing is impossible as is.

maxsono commented 4 years ago

Any progress or alternatives? As the Joi DSL is one of the nicest forms to describe data I have encountered so far, deriving all my types from it would be awesome. It worked perfect with Joi 15, however we moved to Joi 16 already and currently avoid joi-extract-type. @Marsup: Wouldn't joi-extract-type be a feature that would very well fit into Joi itself? Joi is incredibly powerful when used together with TypeScript!.

Marsup commented 4 years ago

I don't have the bandwidth nor the money to support such an effort. Types in general have a place in joi, Eran is currently running a crowdfunding campaign for hapi as a whole (as an experiment) to add types. I don't know how far those types will go, if they'll be basic or advanced, so when/if this campaign is successful, I'd suggest approaching Eran to suggest improvements or work with him towards that goal.

o-alexandrov commented 4 years ago

Unfortunately, it seems the crowdfunding campaign won't reach the goal, so the typings won't be proposed as part of the @hapi/joi itself in the near future.

Could you please clarify:

Let me also attempt to answer the question above based on what I read in the discussions. The decision is to move joi-extract-type typings to be part of the @types/hapi__joi.

guyellis commented 4 years ago

@RecuencoJones @SimonSchick @Marsup @jaulz - you are the owners of @types/hapijoi - is there any initial objection to doing a PR on DefinitelyTyped to merge this functionality into @types/hapijoi?

If not then I can try and rally a mob session on Zoom where those interested can join me and others on making this work. Or if somebody already has something that is ready to go? @TCMiranda - did you already do an experimental merge into the DefinitelyTyped repo?

ChrisCrewdson commented 4 years ago

I created a PR to try to get this library to work in isolation: PR 28 There is a remaining typing issue with the when conditional schema extension that I have not been able to decipher. @TCMiranda maybe you have a suggestion to fix it?

guyellis commented 4 years ago

And I've also thrown that error onto Stack Overflow to see if I can get help there: https://stackoverflow.com/questions/59976848/typescript-interface-incorrectly-extends-interface

TCMiranda commented 4 years ago

Hey guys, I'm stuck at the "namespace issue" I explained here: https://github.com/TCMiranda/joi-extract-type/pull/28#issuecomment-580277415

I'm considering to change the approach to one that "forks" the currently DefinitelyTyped definition and works as a replacement. Any opinion on that?

guyellis commented 4 years ago

I'm considering to change the approach to one that "forks" the currently DefinitelyTyped definition and works as a replacement. Any opinion on that?

I definitely think that would be the easier approach.

There are some other moving pieces. I believe that it is the Hapi community intention to bring the types into the Joi repo. If that happens and they don't copy the types directly from DefinitelyTyped or if they rewrite Joi in Typescript then it's possible that you work would get "left behind."

Were you thinking of doing a PR against DefinitelyTyped to merge in joi-extract-type to Joi's definition or maintaining another definition alongside it and keeping it up-to-date with the original?

typeofweb commented 4 years ago

I believe, given the state of things, merging this effort into the official @types/hapijoi would be the best solution. However, the problem is there are a few other @types/hapi* packages that depend on Joi so such changes to Joi typings would likely also break the other packages. See: https://github.com/DefinitelyTyped/DefinitelyTyped/pull/41976#issuecomment-635437252

o-alexandrov commented 4 years ago

If you don't depend on the vast Joi API, as we don't in our projects, you should consider to checkout:

TCMiranda commented 4 years ago

Seems like a very nice alternative. Unfortunately, I'm not actively working to make this fit on Joi 16, so if that's a blocker for anyone, you should definitely look out there since there are some other libraries that do the same.

Although I like very much this repo approach, which is cleaner and smaller.


For Joi 16 the real issue is we are having trouble to patch the Joi namespace.

The only solution I see now is this one:

I'm considering to change the approach to one that "forks" the currently DefinitelyTyped definition and works as a replacement.

But this also means to stay up-to-date with Joi upgrades which is thought for me since I'm not using Joi on a daily basis anymore. Anyone up to help with this, the steps I see are:

  1. Adding a definition file for Joi on the root of this repository based on DefinitelyTyped;
  2. Change the dependency from @types/hapi__joi to our own version;
  3. Adapt it to allow joi-extract-type to patch it;
  4. Update joi-extract-type to match newer API usage.

I think that continue using the "patching" strategy is good for now to make maintenance easier, as we could replace the Joi definition file without touching the extraction API.

panzelva commented 3 years ago

Hello, if anyone ends up here from google, I managed to make this package work me. I am using:

I simply took index.ts, edited it little and placed it into @types/joi.d.ts in my project (and also edited typeRoots in my tsconfig.json) https://gist.github.com/panzelva/b0e565870c950c4cfd77188d69a3064a

IDK if it will solve this issue but it works quite well for me.

jmikrut commented 3 years ago

@panzelva — your Gist worked for me. I am grateful.

Would love to see Joi 16+ supported but this works great for now.

Thank you!

AverageHelper commented 3 years ago

Popping in to report that @panzelva's gist still works wonders. Plopped it in, and now our project uses modern Joi syntax with full TypeScript inference! (Within reason, obviously, but all of our schemas parse just fine.)

For reference, we're currently using joi@17.3.0 and joi-extract-type@15.0.8. We no longer need to import @hapi/joi anywhere when we have the gist installed with our type declarations.

rossPatton commented 3 years ago

@panzelva just piping to say this really saved me a lot of time! i was in the middle of trying to do this myself and just happened to come across this thread!

pke commented 2 years ago

I am really confused now... hasn't joi been deprectated in favour of @hapi/joi? @panzelva

SimonSchick commented 2 years ago

it was, and then it was-undeprecated and @hapi/joi was deprecated, source of truth is https://www.npmjs.com/package/@hapi/joi

pke commented 2 years ago

@SimonSchick thanks, I should have checked there first but I tried their issue tracker and it had an old, closed and locked ticket recommending to use the hapi prefix one. Thats the thing with locked tickets... you can't amend them. Anyway I moved to zod already quickly as it provides 1st class TS support (even for default values and string unions like "opt1" | "opt"). It lacks automatic number conversation and their default handling is weird too. And they don't have or operators for keys.