fix: passing null object to type array defaults to empty array (#249)

[alemagio]

Checklist

• run npm run test and npm run benchmark
• tests and/or benchmarks are included
• documentation is changed or added
• commit message and code follows the Developer's Certification of Origin
  and the [Code of conduct](https://github.com/fastify/.github/blob/master/CODE_OF_CONDUCT.

[zekth]

lgtm

[RafaelGSS]

Not sure if this is unexpected behavior. The following snippet sent an empty array by default

const schema = {
  type: 'object',
  properties: {
    arr: {
      type: 'array',
      items: {
        type: 'number'
      },
      default: [],
    }
  }
}

const stringify = fastJson(schema)
const valueStr = stringify({}) // {"arr":[]}

Although throwing an error doesn't look good.

[alemagio]

@RafaelGSS Added a unit test with your case.
I'm not having any error, am I missing somenthing?

[RafaelGSS]

No, I said the current implementation(master) seems to have expected behavior since null is sent. I'll wait for some contributors to give their opinion.

Btw the last test case is not necessary, is already being tested here

[alemagio]

No, I said the current implementation(master) seems to have expected behavior since null is sent. I'll wait for some contributors to give their opinion.

Btw the last test case is not necessary, is already being tested here

Oh ok.
I'll remove last test in the meantime.

[mcollina]

Are you sure passing null for the array would validate this schema? Can you check with ajv?

My understanding is that null is not a valid array.

[zekth]

const schema = {
  type: 'object',
  properties: {
    foo: {
      $id: '#/properties/foo',
      type: 'array',
      default: [],
      items: {
        $id: '#/properties/foo/items'
      }
    }
  }
}
const obj = {
  foo: null
}
var Ajv = require('ajv')
var ajv = new Ajv()
var validate = ajv.compile(schema)
var valid = validate(obj)
if (!valid) console.log(validate.errors)
// output
// [
//   {
//     keyword: 'type',
//     dataPath: '.foo',
//     schemaPath: '#/properties/foo/type',
//     params: { type: 'array' },
//     message: 'should be array'
//   }
// ]

Indeed

[alemagio]

I followed the issue, too strictly actually.
Tried with ajv and null is not valid when type is array.
I think a good way to handle this is to throw an error with a more informative message.
What do you think?

[mcollina]

I think a good way to handle this is to throw an error with a more informative message.

Yes, go for it!

[alemagio]

Updated the code.
Now in case something expected to be an array is something else a clearer error is thrown.

[mcollina]

lgtm

[alemagio]

@mcollina Would you mind adding hacktoberfest-accepted label to this PR so I can complete my Hacktoberfest? 😄

[alemagio]

Thanks 😁