Ff00ff/mammoth

The return type of (select|insert etc.).then is inferred as Promise<any>

Closed this issue · 21 comments

...which kind of defeats the purpose of typed queries :D.

Can you give me some more info please?

It's sometimes possible for return types to become any within Mammoth, but there should be plenty of checks (type tests) covering this.

It's also possible because of misconfiguration in your project e.g. not having tsconfig.json setup correctly, or having an error in one of your tables, which causes an any down the line when executing a query.

see: https://github.com/Ff00ff/mammoth/blob/master/src/select.ts#L99

The return type of then is inferred from the onFulfilled function i guess.

i guess any should be a type parameter there

Ah right, yes, this needs to be improved. So just to get the whole situation here, you're probably doing something like this?

execSelectQuery().then((rows) => {
  // at this stage rows is typed correctly
  return 123;
}).then((result) => {
  // and here result is any instead of the proper return type of the onFulfilled
});

yes, and i have the same problem with async/await:

const x = await db
    .insertInto(db.question)
    .values({
      text,
      questionerId
    })
      .returning("id");
return x; // x is typed as any here

OK, I'll get the then() bit fixed.

The async/await case seems different though. Can you share some more info there? Or double-check if there is no error in your question table or somewhere else in your db instance.

Hi, it might be be related to returning, let me know if i'm doing something wrong here:

import * as mam from "@ff00ff/mammoth";

const check = async () => {
  const question = mam.defineTable({
    id: mam.uuid().primaryKey().default(`gen_random_uuid()`),
    text: mam.text().notNull(),
  });

  const db1 = defineDb({ question }, undefined as any); // queryExecutor is not relevant for typing

  const res = await db1.insertInto(db1.question).values({
    text: "hello"
  }).returning("id");

  return res; // typed as any
}

I might have messed up my project settings. Let me double check because vs-code tells me otherwise than my local tsc.

ok, i figured out that my ide used typescript 4.0.2. Using 4.1.2 seems to fix this problem. Sorry for the trouble.

No problem. The then() case is a nice report that needs fixing anyway, thanks. 👍

Ah i understand, so async/await infers the resulting type from the onfulfilled parameter ;). Good to know.

Something else, the type definition of onrejected needs to be fixed too, void is wrong here. See the typescript promise definition here:

/**
 * Represents the completion of an asynchronous operation
 */
interface Promise<T> {
    /**
     * Attaches callbacks for the resolution and/or rejection of the Promise.
     * @param onfulfilled The callback to execute when the Promise is resolved.
     * @param onrejected The callback to execute when the Promise is rejected.
     * @returns A Promise for the completion of which ever callback is executed.
     */
    then<TResult1 = T, TResult2 = never>(onfulfilled?: ((value: T) => TResult1 | PromiseLike<TResult1>) | undefined | null, onrejected?: ((reason: any) => TResult2 | PromiseLike<TResult2>) | undefined | null): Promise<TResult1 | TResult2>;

    /**
     * Attaches a callback for only the rejection of the Promise.
     * @param onrejected The callback to execute when the Promise is rejected.
     * @returns A Promise for the completion of the callback.
     */
    catch<TResult = never>(onrejected?: ((reason: any) => TResult | PromiseLike<TResult>) | undefined | null): Promise<T | TResult>;
}

or to be more precise, all query types should be compatible with the `PromiseLike` interface:

interface PromiseLike<T> {
    /**
     * Attaches callbacks for the resolution and/or rejection of the Promise.
     * @param onfulfilled The callback to execute when the Promise is resolved.
     * @param onrejected The callback to execute when the Promise is rejected.
     * @returns A Promise for the completion of which ever callback is executed.
     */
    then<TResult1 = T, TResult2 = never>(onfulfilled?: ((value: T) => TResult1 | PromiseLike<TResult1>) | undefined | null, onrejected?: ((reason: any) => TResult2 | PromiseLike<TResult2>) | undefined | null): PromiseLike<TResult1 | TResult2>;
}

Yes, this is clear. You seem to have thought this through. Do you want to fix this yourself and adding a quick check (type test) or two? :)

Yes, i can try ;). Are there any existing type tests?

Great! Yes, they are in the __checks__ directory. Mammoth uses dts-jest https://www.npmjs.com/package/dts-jest to snapshot type definitions. This is key to avoid regressions on the public types because it’s so easy to turn everything in any. 😅

How is it going? Do you want me to have a look with you?

Hey, I didn't found the time to start yet. I hope to find some next week.

That's fine. I'm assuming you don't have the time anymore so I'll let someone else pick this one up. Let me know if you already started on this; then it's all yours of course. ;-)

This should be fixed in the latest master. This is not published to npm yet.

@martijndeh thx a lot, i'm sorry that i didn't found the time.

No worries, maybe next time!