Improve `router.resource` type-checking

[webNeat]

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:

import router from '@adonisjs/core/services/router'

class TestController {}

router.resource('test', TestController) // <- TS does not complain

The equivalent code using get, post, ... methods does show Typescript errors

import router from '@adonisjs/core/services/router'

class TestController {}

router.get('/test/create', [TestController, 'create'])  // <- TS complains
router.get('/test', [TestController, 'index'])  // <- TS complains
router.post('/test', [TestController, 'store'])  // <- TS complains
router.get('/test/:id', [TestController, 'show'])  // <- TS complains
router.get('/test/:id/edit', [TestController, 'edit'])  // <- TS complains
router.put('/test/:id', [TestController, 'update'])  // <- TS complains
router.patch('/test/:id', [TestController, 'update'])  // <- TS complains
router.delete('/test/:id', [TestController, 'destroy'])  // <- TS complains

A simple test for this would be:

test.group('Router | typechecking', () => {
  test('router.resource gives Typescript error when a RESTfull method is missing in the given controller', () => {
    class TestControllerClass {}
    const router = new RouterFactory().create()
    router.resource('test', TestControllerClass) // @ts-expect-error
  })
})

Can I make a PR to fix this?

Reproduction repo

No response

[RomainLanz]

Hey @webNeat! 👋🏻

You need to ensure your type will work for any modifier like only(), except() and apiOnly().

[webNeat]

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.

[Julien-R44]

it won't be possible since only(), except() and apiOnly() are called AFTER using resource() method

this means that you won't be able to accumulate type information in a generic. because when resource is called, the type system has no way of knowing that apiOnly will be called next

Or maybe there is a typescript wizardy that i am not aware of !

[webNeat]

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:

https://github.com/webNeat/adonis-ts-plugin-demo/assets/2133333/e96f908b-0159-4f76-b250-fa6026744aba

• Here is a demo repo: https://github.com/webNeat/adonis-ts-plugin-demo
• Here is the Typescript plugin repo: https://github.com/webNeat/adonis-ts-plugin

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 @adonisjs/tsconfig to be included by default on all projects, or should it be a third-party library? Either way, I am ready to work on it :)

Let me know what you think.

[Julien-R44]

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

[thetutlage]

@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.

[Julien-R44]

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

Source : https://github.com/vercel/next.js/blob/b8a7efcf1361da29994247f7d2dd6b58912b6a9e/packages/next/src/server/typescript/index.ts

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

[thetutlage]

@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.

[webNeat]

@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:

1. install it npm i -D adonis-ts-plugin
2. add it to tsconfig.json

   {
   "compilerOptions": {
   // ...
   "plugins": [{ "name": "adonis-ts-plugin" }]
   },
   }

3. Configure VSCode to use the workspace Typescript instead of the global one (as explained in the Nextjs docs):
   • Open the command palette (Ctrl/⌘ + Shift + P)
   • Search for "TypeScript: Select TypeScript Version"
   • Select "Use Workspace Version"

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:

1. it considers all controller methods (it should only consider public methods that match the prototype (ctx: HttpContext, ...args) => any).
2. does not consider inherited methods.
3. requires the argument of only and except to be an array literal, so the following does not work yet:

   
   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.

[thetutlage]

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)

[webNeat]

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.

[thetutlage]

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 :)

[webNeat]

Hello @thetutlage

No worries, take your time in testing it and let me know what you find :)

[ThisIsMissEm]

I think maybe one way you could do this is to support:

router.resource('posts', { only: [ ... ] })
router.resource('posts', { except: [ ... ] })

As a syntax? That way the ResourceRouter's buildRoutes can look to see which it should create.

Right now if I want a resourceful route but without forms, I need to manually do the router.get, router.post, etc...