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

Shorthand bypass flag (required to implement JTD and other schema formats support) #3156

Merged
merged 14 commits into from
Jun 29, 2021

Conversation

josh-hemphill
Copy link
Contributor

@josh-hemphill josh-hemphill commented Jun 24, 2021

Prerequisite for further JTD Schema integration

It allows users that need it, to manually substitute a JTD instance of AJV into instances of Fastify, or create a ValidatorCompiler that parses anything other than JSON Schema, by preventing Fastify from mutating schemas internally if they don't match JSON Schema root properties.

The result of the work on #3096 but PRed against main.
Related #3083

Checklist

@josh-hemphill josh-hemphill changed the title Shorthand bypass flag for Shorthand bypass flag for basic JTD support Jun 24, 2021
build/build-validation.js Show resolved Hide resolved
docs/Server.md Outdated Show resolved Hide resolved
docs/Server.md Outdated Show resolved Hide resolved
Co-authored-by: James Sumners <james@sumners.email>
Copy link
Member

@zekth zekth left a comment

Choose a reason for hiding this comment

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

Minor comment for documentation format consistency.

docs/Server.md Show resolved Hide resolved
docs/Server.md Outdated Show resolved Hide resolved
test/schema-special-usage.test.js Outdated Show resolved Hide resolved
test/schema-special-usage.test.js Outdated Show resolved Hide resolved
josh-hemphill and others added 4 commits June 28, 2021 09:39
Co-authored-by: Vincent LE GOFF <vince.legoff@gmail.com>
Co-authored-by: Vincent LE GOFF <vince.legoff@gmail.com>
@josh-hemphill
Copy link
Contributor Author

josh-hemphill commented Jun 28, 2021

I removed the JTD tests at Eomm's suggestion, since they were only loosely coupled, relying on external dependencies, and I now believe don't make sense in this specific PR, as I mentioned in the comments.

I wanted to have a test that implemented JTD, but now I'm thinking it is indeed a bit misplaced since this PR isn't specifically adding JTD support anyway, testing one of many possible use cases probably isn't the right test to have there.

@josh-hemphill josh-hemphill changed the title Shorthand bypass flag for basic JTD support Shorthand bypass flag (required to implement JTD and other schema formats support) Jun 28, 2021
docs/Server.md Show resolved Hide resolved
Co-authored-by: Vincent LE GOFF <vince.legoff@gmail.com>
Copy link
Member

@zekth zekth 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

@Eomm could you please take a final look before landing?

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

Copy link
Member

@Eomm Eomm left a comment

Choose a reason for hiding this comment

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

I removed the JTD tests at Eomm's suggestion, since they were only loosely coupled, relying on external dependencies, and I now believe don't make sense in this specific PR, as I mentioned in the comments.

thanks @josh-hemphill

The commit history will be helpful for the jtd anyway 👍🏽
We have explored that this is doable in this way

@Eomm Eomm added the semver-minor Issue or PR that should land as semver minor label Jun 29, 2021
@Eomm Eomm merged commit 2337194 into fastify:main Jun 29, 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 Jun 30, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
semver-minor Issue or PR that should land as semver minor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants