Skip to content

Conversation

@sinclairzx81
Copy link
Contributor

This PR adds support for empty {} any schemas to resolve issue #255. This update checks for the existence of a type property on the schema, and if not found, defaults to a JSON.stringify() for serialization.

Have added a few tests to any.test.js and updated index.d.ts accordingly. Submitting this PR for review.

benchmark

cpu  AMD Ryzen 7 3700X 8-Core Processor
ram  32GB 
node v12.18.2

FJS creation x 56,104 ops/sec ±0.26% (92 runs sampled)
CJS creation x 162,430 ops/sec ±0.27% (98 runs sampled)

JSON.stringify array x 3,320 ops/sec ±0.33% (91 runs sampled)
fast-json-stringify array x 8,465 ops/sec ±0.37% (90 runs sampled)

compile-json-stringify array x 7,756 ops/sec ±0.75% (94 runs sampled)
JSON.stringify long string x 10,028 ops/sec ±0.17% (95 runs sampled)

fast-json-stringify long string x 10,098 ops/sec ±0.13% (96 runs sampled)
compile-json-stringify long string x 10,108 ops/sec ±0.10% (99 runs sampled)

JSON.stringify short string x 8,700,507 ops/sec ±0.46% (91 runs sampled)
fast-json-stringify short string x 47,164,570 ops/sec ±0.21% (96 runs sampled)

compile-json-stringify short string x 40,127,602 ops/sec ±0.38% (97 runs sampled)
JSON.stringify obj x 1,827,988 ops/sec ±0.12% (96 runs sampled)

fast-json-stringify obj x 17,418,215 ops/sec ±0.39% (95 runs sampled)
compile-json-stringify obj x 24,095,360 ops/sec ±0.23% (95 runs sampled)

Checklist

Copy link
Member

@fox1t fox1t left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@L2jLiga L2jLiga left a comment

Choose a reason for hiding this comment

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

LGTM

@mcollina
Copy link
Member

mcollina commented Jan 8, 2021

cc @jsumners

@sinclairzx81 sinclairzx81 changed the title Support for empty Any schemas #255 Support for empty schemas #255 Jan 9, 2021
Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina merged commit 1c6b4da into fastify:master Jan 15, 2021
@sinclairzx81
Copy link
Contributor Author

@mcollina thanks :)

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.

5 participants