fix: check all types for toJSON function

[ivan-tymoshenko]

Checklist

• [x] run npm run test and npm run benchmark
• [x] 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

toJSON function can return any type as a result. Not it's working only if object has toJSON method that returns another object.

There is a problem with the Date type. It has toJSON function by default, and it's not clear how to handle this case.

[mcollina]

Does this causes any perf regression?

[ivan-tymoshenko]

[ivan-tymoshenko]

It's an interesting example https://github.com/fastify/fast-json-stringify/blob/03f8714fc2412c3e46cb7bbb947e316cad0681bc/test/date.test.js#L129

moment is an object. It's not an instance of Date . It has toJSON method. Should we call it?

[mcollina]

The test is correct.

[mcollina]

Is this still in the works? What's meeting?

[ivan-tymoshenko]

Yes, it's WIP.

1. I need to fix toJSON check in asTypeNullable functions. toJSON should be called before checking for a null value, but we shouldn't call toJSON twice: in the asTypeNullable function and asType function. This means we should rewrite the way how we check for a null value.
2. Add toJSON check for Date type, if the value is not a Date object.
3. Add toJSON check before all types of type checking: anyOf, oneOf, property type is an array, additional fields, etc.

[ivan-tymoshenko]

If someone would like to help with this issue, I would appreciate it. Working on another problem right now.

[bmenant]

@ivan-tymoshenko Did you see https://github.com/fastify/fast-json-stringify/pull/414 ? I tried to tackle this as well, started with pretty much the same changes you did so far, and went even further way, before eventually giving up:

Turns out this is far tougher than expected. Notably, the logic laid out toward the nested() function requires too many conditions and exceptions, preventing easy and safe (and fast?) behavior addition such as this one.

IMHO, a thorough refactoring is needed in and around the nested() function before. But, to be honest, it’s not easy task to grasp the whole logic of the code builder. It has been written to output optimized code from the ground up for a reason. I personally lack the skills and the time to tackle this.

After all, AFAIK none of the fast json stringify lib I tried out there supports such toJSON feature.

Anyway, I’d be glad to discuss the topic and give a hand if you deem so useful.