elysiajs / elysia

Ergonomic Framework for Humans
https://elysiajs.com
MIT License
10.6k stars 226 forks source link

Typebox Boolean regression in 1.1.17 #872

Open ryanleecode opened 1 month ago

ryanleecode commented 1 month ago

What version of Elysia is running?

1.1.17

What platform is your computer?

Linux 6.6.52-1-MANJARO x86_64 unknown

What steps can reproduce the bug?

Create api route where query parameter has a boolean

for example

query: t.Object({
  registered: t.Optional(
    t.Boolean({
      description: 'Filter usernames by registration status',
    }),
  ),
}),

Submit request and do not include the boolean value, in this case omit registered

What is the expected behavior?

it works

What do you see instead?

Tybebox Error

Additional information

No response

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

No response

aburii commented 1 month ago

I've been experiencing the same when using optional booleans in query parameters

image
ilijapuaca commented 1 month ago

Same issue here, was running 1.1.16 so the issue was likely introduced in an earlier version. Tried upgrading but it's still there, defining an optional boolean in a query is always being treated as required

crishoj commented 2 weeks ago

Can you provide a minimal example to reproduce the issue?

Optional boolean query parameters seem to work in Elysia 1.1.23:

import Elysia, {t} from 'elysia'

const app = new Elysia()
    .get('/test', ({query: {registered}}) => {
        return `Registered: ${registered}`
    }, {
        query: t.Object({registered: t.Optional(t.Boolean())}),
    })

const urls = [
    'https://example.com/test?registered=true',
    'https://example.com/test?registered=false',
    'https://example.com/test'
]

for (const url of urls) {
    const res = await app.handle(new Request(url))
    console.log(url, '→', await res.text())
}

Output:

https://example.com/test?registered=true → Registered: true
https://example.com/test?registered=false → Registered: false
https://example.com/test → Registered: undefined
ilijapuaca commented 2 weeks ago

@crishoj I almost gave up trying to reproduce it as your snippet seems to work well, but if you add another required property to the query object it fails with a boolean validation error

crishoj commented 2 weeks ago

@ilijapuaca Bingo — reproducible when second query param is added to the schema:

import Elysia, {t} from 'elysia'

const app = new Elysia()
    .get('/test', ({query: {registered}}) => {
        return `Registered: ${registered}`
    }, {
        query: t.Object({
            registered: t.Optional(t.Boolean()),
            other: t.String(),
        }),
    })

const urls = [
    'https://example.com/test?other=foo&registered=true',
    'https://example.com/test?other=foo&registered=false',
    'https://example.com/test?other=foo'
]

for (const url of urls) {
    const res = await app.handle(new Request(url))
    console.log(url, '→', res.status, await res.text())
}

Output

image
crishoj commented 2 weeks ago

The issue appears to only affect ahead-of-time optimized handlers, which is Elysia's default:

image
crishoj commented 2 weeks ago

Possible fix for the issue: #907

I am not sure I understand the intended behavior of BooleanString.Decode well enough, though, so hoping @bogeychan will chime in here.