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

Add support for transform types #127

Closed
wants to merge 3 commits into from

Conversation

ehaynes99
Copy link

@ehaynes99 ehaynes99 commented Feb 8, 2024

fixes #125

Checklist

  • run npm run test and npm run benchmark (tests pass, but no benchmark script exists)
  • tests and/or benchmarks are included
  • documentation is changed or added (I don't think this needs an update)
  • commit message and code follows the Developer's Certification of Origin
    and the Code of conduct

@ehaynes99 ehaynes99 changed the title Add support for transform types Add support for transform types (fixes #125) Feb 8, 2024
@ehaynes99 ehaynes99 changed the title Add support for transform types (fixes #125) Add support for transform types (fixes #125 ) Feb 8, 2024
@ehaynes99 ehaynes99 changed the title Add support for transform types (fixes #125 ) Add support for transform types Feb 8, 2024
@mcollina
Copy link
Member

mcollina commented Feb 8, 2024

Could you adjust the versions in CI?

@ehaynes99
Copy link
Author

Could you adjust the versions in CI?

Ah, sorry, didn't know that existed.

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.

What would happen if a TransformType is used with the stock validator compiler?

@ehaynes99
Copy link
Author

What would happen if a TransformType is used with the stock validator compiler?

Hrmm, yeah, I guess that won't work. Honestly I've just always used these together. I guess that means that this would need two independent type providers, e.g. TypeBoxTypeProvider and TypeBoxTransformTypeProvider, and require the user to pick based on whether the validator compiler is used or not.

Regarding the tests, I'll try to track down which specific version had StaticDecode. I thought it was in 0.30.0 based on the changelog, but apparently not.
https://github.com/sinclairzx81/typebox/blob/master/changelog/0.30.0.md

I'll close this for now and try to come up with something else. It'll probably be a few days before I can get back to it though.

@ehaynes99 ehaynes99 closed this Feb 12, 2024
@tothdanielax
Copy link

@ehaynes99, followed your setup as it is super handy for our scenario. But it seems the conversion works only for query, params and headers. Is there a way to decode the body also (I receive ids represented as strings and it's needed to be converted to Mongo ObjectIds)?

@ehaynes99
Copy link
Author

@tothdanielax Hrmm, I'm not sure how that's possible. The test I added for 'should decode transform types' actually only tests the body conversion, not query or path, so if anything wasn't working, I would expect it to be one of the others.

The TypeBox author added a PR for this a while ago, but ended up closing it because of the version range issue. That's probably a better way forward because it handles the case @mcollina mentions as it precludes the combination of transform types without transforms in the validator compiler.

Sorry, I've been pretty swamped, but I'll try to resurrect that one instead.
See:
sinclairzx81/typebox#343 (comment)

I think the syntax can easily be adapted to have flags for the other two new options (though i guess that would prompt an even narrower version range, so we could add it separately).

Using the same parameter names from ajv, that could look something like:

const fastify = TypeBoxTransformProvider(Fastify(), {
  removeAdditional: true,
  useDefaults: true,
})

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.

Support for transform types
3 participants