Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

docs: Extend TypeBox documentation #3437

Merged
merged 5 commits into from
Nov 13, 2021
Merged

docs: Extend TypeBox documentation #3437

merged 5 commits into from
Nov 13, 2021

Conversation

Jnig
Copy link
Contributor

@Jnig Jnig commented Nov 12, 2021

Hi,

the following PR closes #3421 and adds additional documentation how to use TypeBox with fastify v4.

Checklist

@github-actions github-actions bot added the documentation Improvements or additions to documentation label Nov 12, 2021
ajv: {
customOptions: {
strict: 'log',
keywords: ['kind', 'modifier'],
Copy link
Member

Choose a reason for hiding this comment

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

would be great to inform in which cases specifying the keywords is needed.

Copy link
Contributor Author

@Jnig Jnig Nov 12, 2021

Choose a reason for hiding this comment

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

Wow that was quick :) I've added the following informations:

TypeBox uses the properties kind and modifier internally. Those are not strictly valid JSON schema and will throw an AJV error. To remove the error it's either necessary to omit the properties by using Type.strict() or use the AJV options for adding custom keywords.

@Jnig
Copy link
Contributor Author

Jnig commented Nov 12, 2021

cc @sinclairzx81 :)

@@ -85,4 +85,6 @@ server.get('/route', {
})
```

TypeBox uses the properties `kind` and `modifier` internally. Those are not strictly valid JSON schema and will throw an AJV error. To remove the error it's either necessary to omit the properties by using [`Type.strict()`](https://github.com/sinclairzx81/typebox#strict) or use the AJV options for adding custom keywords.
Copy link
Contributor

Choose a reason for hiding this comment

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

Type.strict() should be capitalized as Type.Strict()

Copy link
Contributor

Choose a reason for hiding this comment

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

Possibly rephrase

Those are not strictly valid JSON schema and will throw an AJV error

as

These properties are not strictly valid JSON schema which will cause AJV to throw an invalid schema error

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks! Comments are incorporated now :)

@sinclairzx81
Copy link
Contributor

@Jnig Looks good to me. Nice work :)

Copy link
Member

@RafaelGSS RafaelGSS left a comment

Choose a reason for hiding this comment

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

@sinclairzx81 I'm assuming that those changes are also necessary if the user would like to work with typebox (even without type-providers) on v4, is that right?

I'm wondering if we can do something to make it built-in in order to avoid the needs of the user to set up it by themselves.

@sinclairzx81
Copy link
Contributor

sinclairzx81 commented Nov 12, 2021

@RafaelGSS Hi. The requirement to specify the kind and modifier keywords is specific to AJV version 7 onwards which mandates strict keyword checking by default. Note, this is unrelated to the Type Provider implementation which is tied to TypeScript types only.

There's only two options I can think of to make this automatic for users:

  1. Automatically configure the AJV compiler in Fastify for the kind and modifier keywords.
  2. Automatically serialize / deserialize schemas prior to passing over to the AJV compiler.

Option 1 is not really workable as it would require Fastify to have knowledge of an external libraries internals. But Option 2 could be worth exploring.

The Type.Strict() function in TypeBox essentially does a JSON.parse(JSON.stringify(schema)) to strip out these properties. The kind and modifier properties are defined as Symbol values specifically to permit this logic, with this largely in response to AJV 7's decision to mandate strict checks.

In the context of Fastify, internally implementing this serialize / deserialize pass to clean schemas prior to AJV compilation may prove workable given that schema compilation only happens once at start up. Such a step would permit users an escape hatch to include additional metadata on their schemas, (with the rule being that the metadata MUST NOT be serializable to JSON). I wouldn't expect the serialize/deserialize step to be much of an added cost over the existing AJV compilation pass (tho it would be worth measuring for larger schemas).

Hope this helps!

Copy link
Member

@RafaelGSS RafaelGSS left a comment

Choose a reason for hiding this comment

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

Let's explicitly define the version of ajv which those settings are needed.

docs/Type-Providers.md Outdated Show resolved Hide resolved
@Jnig
Copy link
Contributor Author

Jnig commented Nov 12, 2021

@sinclairzx81 I'm assuming that those changes are also necessary if the user would like to work with typebox (even without type-providers) on v4, is that right?

I'm wondering if we can do something to make it built-in in order to avoid the needs of the user to set up it by themselves.

Having a solution which works without additional configuration would be perfect. Than it would be the same as for fastify 3.

Jnig and others added 2 commits November 13, 2021 00:36
Co-authored-by: Rafael Gonzaga <rafael.nunu@hotmail.com>
Copy link
Member

@RafaelGSS RafaelGSS 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 795e83a into fastify:next Nov 13, 2021
@github-actions
Copy link

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 14, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants