Skip to content

Add support for binary type#226

Merged
hueniverse merged 7 commits intohapijs:masterfrom
petreboy14:master
Apr 18, 2014
Merged

Add support for binary type#226
hueniverse merged 7 commits intohapijs:masterfrom
petreboy14:master

Conversation

@petreboy14
Copy link
Contributor

Wanted to add support for validating objects with properties of type Buffer.

@petreboy14
Copy link
Contributor Author

Updated to be inline with current implementation using Hoek instead of Utils for inherits and language.js instead of languages/en_us.json

@hueniverse
Copy link
Contributor

What are you doing that requires Buffer validation? joi was created as a schema validation tool. I am having a hard time extending it to various JS or node types that cannot be represented in any other way than native code.

@petreboy14
Copy link
Contributor Author

There’s a database module that requires Buffers for Binary types and i’m writing a parameter checking layer on top of the database module. I was hoping to avoid having to convert the Buffer to a string and then back again.

Would it be better to check if Buffer exists and return an error if it does not if trying to validate a Buffer object in a non-node environment?

On Apr 8, 2014, at 10:16 PM, Eran Hammer notifications@github.com wrote:

What are you doing that requires Buffer validation? joi was created as a schema validation tool. I am having a hard time extending it to various JS or node types that cannot be represented in any other way than native code.


Reply to this email directly or view it on GitHub.

@hueniverse
Copy link
Contributor

Hmm. I might be open to some kinds of general purpose type() method on any() that will also cover regex or other native types for node and JS. But I am worried about starting down the path of supporting internal types, especially node.

@petreboy14
Copy link
Contributor Author

That could work and would be helpful for custom types. Or would you restrict it to core native node types only?

On Apr 8, 2014, at 10:30 PM, Eran Hammer notifications@github.com wrote:

Hmm. I might be open to some kinds of general purpose type() method on any() that will also cover regex or other native types for node and JS. But I am worried about starting down the path of supporting internal types, especially node.


Reply to this email directly or view it on GitHub.

@hueniverse
Copy link
Contributor

Not sure. Depends how it is implemented.

@hueniverse
Copy link
Contributor

Ok, after some more thinking, I like the idea of adding a binary() type that takes either a Buffer or string (which gets converted to a Buffer). The master branch changes a lot for v4 so this will need to be reworked if you are still interested in adding it.

@hueniverse hueniverse changed the title Add support for Buffer Add support for binary type Apr 17, 2014
@petreboy14
Copy link
Contributor Author

Sure, I'll make this compatible with latest master

@petreboy14
Copy link
Contributor Author

Updated for latest version of master. Do you want me to add in code that checks for existence of Buffer before trying to convert a string to a buffer in the case of non-node environments?

@hueniverse
Copy link
Contributor

Let's see what Browserify does...

hueniverse pushed a commit that referenced this pull request Apr 18, 2014
Add support for binary type
@hueniverse hueniverse merged commit 571bc0d into hapijs:master Apr 18, 2014
@hueniverse hueniverse added this to the 4.0.0 milestone Apr 18, 2014
@hueniverse hueniverse self-assigned this Apr 18, 2014
hueniverse pushed a commit that referenced this pull request Apr 18, 2014
@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

feature New functionality or improvement

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants