-
-
Notifications
You must be signed in to change notification settings - Fork 32
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
Improve router.resource
type-checking
#81
Comments
Hey @webNeat! 👋🏻 You need to ensure your type will work for any modifier like |
Thanks for the remark @RomainLanz, I missed those. I see the difficulty now. I will give it a shot and let you know if I could solve it. |
it won't be possible since this means that you won't be able to accumulate type information in a generic. because when Or maybe there is a typescript wizardy that i am not aware of ! |
You are right @Julien-R44, I didn't find any way to do it using only normal types. So I made it possible using a Typescript plugin: adonis-ts-plugin.mp4
This plugin is still a POC (missing some edge cases, tests, ...), There might be other typechecking improvements that can be added to the Adonis framework using this approach. My question now is whether this plugin should be part of Let me know what you think. |
that looks cool dude, well done. making a typecript plugin really doesn't look easy ahah my two cents on this: with V6, we've finally been able to get rid of our custom compiler. we are now only using standard stuff that is supported everywhere i think this is the right way to go. Using a custom ts plugin for typechecking seems a bit twisted to me, and if we want to have stricter typechecking on the router, then we should instead redesign the router API, rather than developing a non-standard typechecking plugin and having to maintain it |
@webNeat That looks impressive. First, I want to understand how TypeScript plugins are applied and are they picked by all the editors using TypeScript LSP? If yes, then I might be tempted to use it. Because, @Julien-R44 if we change router API, then we will introduce a breaking change, something I would like to avoid. So yeah let's explore and see what is best in this case and keep all options open. |
after some research, I noticed that Next.JS also offers a typescript plugin: Doc : https://nextjs.org/docs/app/building-your-application/configuring/typescript#typescript-plugin In fact, im afraid that it's looks too "magical", or that it will cause errors that are hard to understand because nobody's ever seen them, because specific to our plugin but my fear probably comes from the fact that i'm not familiar with typescript plugins and have never used one |
@webNeat How about releasing this plugin. Let us use it for a while and provide you the feedback. If everything feels smooth, we can go ahead and add it to the default config. |
@thetutlage That works for me. The alpha version I used in the demo is already on npm https://www.npmjs.com/package/adonis-ts-plugin Here is how to use it:
{
"compilerOptions": {
// ...
"plugins": [{ "name": "adonis-ts-plugin" }]
},
}
I will try to test other editors and see how it works with them. Here is the list of known bugs/limitations, in this alpha version, I am woking on:
const ignoredRoutes = ['edit', 'destroy']
const storeRoute = 'store'
router.resource('demo', DemoController)
.only([storeRoute, 'destroy']) // <- doesn't know that `storeRoute` is `'store'`
router.resource('demo', DemoController)
.except(ignoredRoutes) // <- doesn't resolve the value of `ignoredRoutes` Let me know if you found any other bugs. |
Cool. So yeah, it seems like there are some usability issues right now. But, I will still give it a try personally and report other issues (if I find any) |
Hello @thetutlage, it took me some time but I finally fixed all the issues above and released the version 1.0.0 of the plugin. https://github.com/webNeat/adonis-ts-plugin Feel free to try it and let me know if you find any issues. |
Hey @webNeat Sorry, it took unexpected long to get back to you. Just got busy with too many things. I will give the plugin a try this week and share my feedback here. Thanks a ton for taking out time and working on it :) |
Hello @thetutlage No worries, take your time in testing it and let me know what you find :) |
Package version
7.0.2
Describe the bug
Hello,
I noticed that
router.resource
does not give Typescript error when the given controller does not implement all the needed REST methods.For example, the following code does not show any error but will fail at runtime:
The equivalent code using
get
,post
, ... methods does show Typescript errorsA simple test for this would be:
Can I make a PR to fix this?
Reproduction repo
No response
The text was updated successfully, but these errors were encountered: