Skip to content

feat: support t.Integer()#953

Merged
SaltyAom merged 4 commits intoelysiajs:mainfrom
scarf005:fix/949/`tInteger`-doesnt-work-with-query-params
Dec 23, 2024
Merged

feat: support t.Integer()#953
SaltyAom merged 4 commits intoelysiajs:mainfrom
scarf005:fix/949/`tInteger`-doesnt-work-with-query-params

Conversation

@scarf005
Copy link
Contributor

fixes #949

format: 'integer',
default: 0
}),
t.Number(property)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably a typo? should be Integer and not Number

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

adding t.Integer causes infinite recursion:

 Params Validator > parse malformed integer [3.00ms]
370 |                                   [
371 |                                           t.String({
372 |                                                   format: 'integer',
373 |                                                   default: property?.default ?? 0
374 |                                           }),
375 |                                           t.Integer(property)
              ^
RangeError: Maximum call stack size exceeded.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah using t.Integer will lead to infinite recursion, but you could do Type.Integer.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oops, i've just seen it, noted.

)
.Decode((value) => {
const number = +value
if (isNaN(number)) return value
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nan is not a valid integer usually

Copy link
Contributor Author

@scarf005 scarf005 Dec 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

applied in 3173e52

[
t.String({
format: 'integer',
default: 0
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i know Numeric does this too but shouldn't this be property.default?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this seems to break other tests which I feel are out of scope for this PR:

 Replace Schema Type > replace object properties in Union
 96 |                           {
 97 |                                   from: t.Number(),
 98 |                                   to: () => t.Numeric()
 99 |                           }
100 |                   )
101 |           ).toMatchObject(
          ^
error: expect(received).toMatchObject(expected)

  {
    [Symbol(TypeBox.Kind)]: "Object",
    properties: {
      id: {
        [Symbol(TypeBox.Kind)]: "Union",
        [Symbol(TypeBox.Transform)]: {
          Decode: [Function],
          Encode: [Function],
        },
        anyOf: [
          {
            [Symbol(TypeBox.Kind)]: "String",
+           default: 0,
-           default: 1,
            format: "numeric",
+           title: "hello",
            type: "string",
          },
          {
            [Symbol(TypeBox.Kind)]: "Number",
            default: 1,
            title: "hello",
            type: "number",
          }
        ],
        default: 1,
        title: "hello",
      },
      name: {
        [Symbol(TypeBox.Kind)]: "String",
        type: "string",
      },
    },
    required: [
      "id",
      "name"
    ],
    type: "object",
  }

- Expected  - 1
+ Received  + 2

})
.Encode((value) => value) as any as TNumber
},
Integer: (property?: NumberOptions): TInteger => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IntegerOptions doesn't exist?

Copy link
Contributor Author

@scarf005 scarf005 Dec 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

applied in 3173e52

if (!FormatRegistry.Has('integer'))
FormatRegistry.Set(
'integer',
(value) => !!value && !isNaN(+value) && Number.isInteger(+value)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Number.isInteger already check for NaN values.

Copy link
Contributor Author

@scarf005 scarf005 Dec 20, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also applied in 3173e52

Co-authored-by: Zoe Roux <zoe.roux@zoriya.dev>
@SaltyAom SaltyAom merged commit 489fb57 into elysiajs:main Dec 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

t.Integer() doesn't work with query params

3 participants