TS errors if model methods are not async

[z1haze]

According to the docs model methods are supposed to support either sync or async, but in practice when I attempted to, for example, write the generateRefreshToken in a synchronous function, typescript errors because it isn'y async. So i peaked into the interface that the source validates against, and all of the methods are written to return a promise.

Documentation stating that they can be sync (supposedly)

Version >=5.x: Callback support has been removed! Each model function supports either sync or async (Promise or async function) return values.

Source showing it expects it to return a promise

generateRefreshToken?(client: Client, user: User, scope: string[]): Promise<string>;

[jankapunkt]

@z1haze thanks for reporting this. I think the docs are just confusing here. The lib treats every model function as async, which in turn of course allows to also pass sync methods, as they are resolved within an async context and are thus non-blocking.

I created a PR as initial input for further discussion: #347

[dhensby]

I think it's important to note here that typescript is just a superset of javascript and the documentation is not wrong here. A sync method can absolutely be used, however typescript is doing its job by providing a narrower typed interface on top of javascript.

For example, as JS is loosely typed various value types can be provided in place of others, numbers instead of bool; strings instead of numbers, etc. Typescript types don't usually enumerate the fact that the internals of their code could accept null or 0 instead of false even though that could technically work; instead they present an idealised interface that is well typed and asks for a bool instead. That's essentially what is happening here; yes you could provide a sync value, but the typings are narrowing the defined interface to Promise only.

I agree it is confusing for a TS developer who is reading the docs to see that the typings don't agree (and we should probably update the docs and say that they should all be Promises so that we can further tighten the implementation later), but ultimately the fix here is trivial, just mark the function async and move on (or if your linter doesn't like this return Promise.resolve(val)).