-
Notifications
You must be signed in to change notification settings - Fork 27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Joi 16? #22
Comments
@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: |
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 https://www.typescriptlang.org/docs/handbook/declaration-merging.html#merging-namespaces Ideas? |
@TCMiranda - feel free to assign this issue to me for now. If I'm unable to do a PR I'll let you know. |
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. |
@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? |
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 |
@guyellis I opened a branch https://github.com/TCMiranda/joi-extract-type/tree/feature/joi16 |
@TCMiranda - I don't see any commits in the joi16 branch beyond Sep 19 (a month ago) - did you push them to GitHub? |
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 |
@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.
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 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 |
A coworker @llwt found the reason why they wrapped the DefinitelyType definitions in a namespace: DefinitelyTyped/DefinitelyTyped#38361 (comment) |
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: #22 (comment) It was awesome that you found the reason of the "extends problem", thanks!
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 |
@TCMiranda - hope you had a good break/vacation wherever you went. Yes - I think that a dummy property will allow 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. |
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? |
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. |
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 DefinitelyTyped/DefinitelyTyped#38361 |
Hi @guyellis, I'm not really sure how you expect these interfaces to be exported, aren't they accessible from the I.e,
|
@RecuencoJones - yes they are accessible from the The issue here is that we are trying to augment and "extend" that typing definition file. So it seems that unexported interfaces inside a namespace cannot be augmented. @TCMiranda - did I summarize that accurately? |
Yes @guyellis you did. @RecuencoJones I sent an example here: |
I see, if I recall correctly there's a problem with exporting multiple variables/interfaces in the root when there's Let me check if it would be possible to somehow have both of them, maybe using |
Edit: nvm, this would break Joi imports, as they have to follow the syntax |
What a pity @RecuencoJones - I was just typing up a response that I thought that it would work - thanks for trying that! |
We need interfaces exported inside the namespace. I think that the problem you refer is for top level exports, right? 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 😄 |
Had to disable the rule strict-export-declare-modifiers but otherwise looks fine: |
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. |
@RecuencoJones thank you, that's awesome! 🎉 @guyellis now we can move on to:
|
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. |
We're also willing to consider changes to joi if typing is impossible as is. |
Any progress or alternatives? As the |
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. |
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.
|
@RecuencoJones @SimonSchick @Marsup @jaulz - you are the owners of @types/hapi__joi - is there any initial objection to doing a PR on DefinitelyTyped to merge this functionality into @types/hapi__joi? 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? |
I created a PR to try to get this library to work in isolation: PR 28 |
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 |
Hey guys, I'm stuck at the "namespace issue" I explained here: #28 (comment) 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? |
I believe, given the state of things, merging this effort into the official @types/hapi__joi would be the best solution. |
If you don't depend on the vast Joi API, as we don't in our projects, you should consider to checkout:
|
Seems like a very nice alternative. 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:
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.
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. |
Hello, if anyone ends up here from google, I managed to make this package work me.
I simply took IDK if it will solve this issue but it works quite well for me. |
@panzelva — your Gist worked for me. I am grateful. Would love to see Joi 16+ supported but this works great for now. Thank you! |
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 |
@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! |
I am really confused now... hasn't |
it was, and then it was-undeprecated and |
@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. |
Does this library have support (or upcoming support) for Joi 16?
The text was updated successfully, but these errors were encountered: