Typecheck breaks for response schema not containing 200 status code

aburii commented 4 weeks ago

What version of Elysia is running?

latest

What platform is your computer?

Darwin 23.5.0 arm64 arm

What steps can reproduce the bug?

import { Elysia, t } from "elysia";

const app = new Elysia()
    .get("/hello", () => "Hello Elysia", {
        response: {
            200: t.String()
        }
    })
    .get('/wrong', () => {
        return { 201: 1 } // Should be marked as error  as below
    }, {
        response: {
            201: t.String()
        }
    })
    .get('/wrong-200', () => {
        return { 200: 1 } // Raises the error
    }, {
        response: {
            200: t.String()
        }
    })
    .listen(3000);

console.log(
  `🦊 Elysia is running at ${app.server?.hostname}:${app.server?.port}`
);

Here is the screenshot of my IDE (WebStorm 2024.2)

What is the expected behavior?

The expected behavior is that all 20(X) (201, 204 and so on) typecheck work.

In this case

    .get('/wrong', () => {
        return { 201: 1 }
    }, {
        response: {
            201: t.String()
        }
    })

Should raise the same error as for 200 status code like:

Details:

What do you see instead?

201 response schema does not trigger typecheck error raise.

Additional information

I am 100% this error does NOT come from my LSP nor my IDE since I have cleaned up the cache and restarted the services multiples times.

It seems that the pasts fixes have been removed (e.g. PR #596 )

Have you try removing the `node_modules` and `bun.lockb` and try again yet?

Yes

bogeychan commented 3 weeks ago

The type behaviour is correct.

201 is the status code, no response body value.

response: {
    201: t.String()
//  ^ status
//            ^ body value
}

Based on your example, read the comments:

new Elysia()
  .get(
    "/hello",
    () => "Hello Elysia", // no error because you return string with 200 status code
    {
      response: {
        200: t.String(),
      },
    }
  )
  .get(
    "/wrong",
    () => {
      return { 201: 1 }; // no error because you implicit return 200 status code with json body of { 201: 1 }
    },
    {
      response: {
        201: t.String(),
      },
    }
  )
  .get(
    "/wrong-200",
    () => {
      return { 200: 1 }; // This raises error because 200 response schema (see below) overwrites the default. and returning here should be string, not object (json)
    },
    {
      response: {
        200: t.String(),
      },
    }
  );

bogeychan commented 3 weeks ago

Use this if you wanna make the last error work:

app.get(
  "/wrong-200",
  () => {
    return { 200: 1 };
  },
  {
    response: {
      200: t.Object({
        200: t.Number(),
      }),
    },
  }
);

bogeychan commented 3 weeks ago

All this kinda follows the OpenAPI responses semantic: https://swagger.io/docs/specification/describing-responses/

aburii commented 3 weeks ago

@bogeychan same issue here on this code

import { Elysia, t } from "elysia";

const app = new Elysia()
    .get("/hello", () => "Hello Elysia", {
        response: {
            200: t.String()
        }
    })
    .get('/wrong', ({ set }) => {
        set.status = 'Created'
        return 1 // Should be marked as error  as below
    }, {
        response: {
            201: t.String()
        }
    })
    .get('/wrong-200', () => {
        return 1 // Raises the error
    }, {
        response: {
            200: t.String()
        }
    })
    .listen(3000);

console.log(
  `🦊 Elysia is running at ${app.server?.hostname}:${app.server?.port}`
);

The first examples I've shown are indeed not correct.

Here is the screenshot of the current error state in my IDE (WebStorm 2024.2)

I have to mention this issue has already been fixed by april but it seems to happen again, please refer to #596 and #490

bogeychan commented 3 weeks ago

@aburii thanks for sharing this finding.

@SaltyAom

The response schema with different status codes, as shown above (https://github.com/elysiajs/elysia/issues/785#issuecomment-2297342653), doesn't work anymore.
And returning number with response schema of t.String shouldn't give a type error, maybe?

aburii commented 3 weeks ago

@aburii thanks for sharing this finding.

@SaltyAom

The response schema with different status codes, as shown above (Typecheck breaks for response schema not containing 200 status code #785 (comment)), doesn't work anymore.

And returning number with response schema of t.String shouldn't give a type error, maybe?

And please ignore the firsts examples I provided, they are incorrect, only the lasts are. By trying to illustrate more on the first examples I've shown no relevant issue.

What I tried to share are in fact the last examples.

SaltyAom commented 2 weeks ago

@bogeychan same issue here on this code
import { Elysia, t } from "elysia";

const app = new Elysia()
    .get("/hello", () => "Hello Elysia", {
        response: {
            200: t.String()
        }
    })
    .get('/wrong', ({ set }) => {
        set.status = 'Created'
        return 1 // Should be marked as error  as below
    }, {
        response: {
            201: t.String()
        }
    })
    .get('/wrong-200', () => {
        return 1 // Raises the error
    }, {
        response: {
            200: t.String()
        }
    })
    .listen(3000);

console.log(
  `🦊 Elysia is running at ${app.server?.hostname}:${app.server?.port}`
);
The first examples I've shown are indeed not correct.

Here is the screenshot of the current error state in my IDE (WebStorm 2024.2)

I have to mention this issue has already been fixed by april but it seems to happen again, please refer to #596 and #490

The /wrong here is not possible due to limitation of TypeScript. As it's not possible to infer the type of set.status = 201, Elysia assume the endpoint returning 200 status code.

To make this possible, Elysia has a dedicated error function to validate type integrity.

So if we refactor the code to use error function as the following:

new Elysia()
    .get('/wrong', ({ error }) => {
        return error(201, 1)
    }, {
        response: {
            201: t.String()
        }
    })
    .listen(3000)

We can see that the IDE now show the error.

So in other words, I recommended to use a dedicated error function instead of using set.status to ensure type integrity as using set.status isn't possible to infer return type due to TypeScript limitation.

aburii commented 2 weeks ago

@SaltyAom Feels a bit weird to call a function 'error' to send back a success status, is there any function planned on next versions of Elysia to feel more accurate ?

elysiajs / elysia