search

jeffbski / joi-browser

joi validation bundled for the browser (deprecated) - use joi@16+
Other
273 stars 39 forks source link

joi 10 #14

Open k-sheth opened 7 years ago

k-sheth commented 7 years ago

Hi,

Joi 10.x is out with joi-date-extensions. Not sure how that will play with joi-browser. Recommendations ??

Thanks,

jeffbski commented 7 years ago

I'm not familiar with the change can you elaborate on what you are concerned with?

jaredhirsch commented 7 years ago

@jeffbski Related question: are you planning to bump the version to catch up with Joi (currently at 10.0.1)?

k-sheth commented 7 years ago

The joi release notes are here - Joi 10.0 release notes

Basically, to get Joi.date.format() related functionality we need to use joi-date-extensions now. joi-date-extensions

My concern is that if we bump to joi-browser to joi 10, we will probably not be able to use joi.date.format and the whole point of joi-browser is sort of lost.

Would i make sense to publish two min files ? one with joi-date-extensions and other without ?

thanks

jeffbski commented 7 years ago

@6a68 yes, I like to keep up as best I can with tracking joi. So we can release a new one here as soon as we figure out how to handle any changes in this major version bump.

@k-sheth thanks for linking to that. I'll take a look at it and see what I can figure out.

jeffbski commented 7 years ago

I guess we have two options here:

  1. create a new joi-date-extensions-browser
  2. create pre-bundled version with or without the date extensions

In weighing the consequences, it seems like maybe option 1 is best. By doing that we are tracking exactly what joi is doing so there should be less confusion.

Also if one is building universal/isomorphic js apps and using the package.json browser map to substitute browser versions from node.js versions then option 1 will work properly since you would map joi to joi-browser and joi-date-extensions to joi-date-extensions-browser and then you would use them the same in your code. If instead we had prebundled then the code would be different.

So I think that adding another package for joi-date-extensions-browser seems like the cleanest approach.

Would that be ok with all of you?

k-sheth commented 7 years ago

@jeffbski : option 1 is cleaner. makes sense to go with that.

jaredhirsch commented 7 years ago

@jeffbski Thanks for the update! Option 1 seems better to me as well.

k-sheth commented 7 years ago

hello, thanks for the update. but it seems joi-extensions-date-browser isn't on github or npm yet.

jeffbski commented 7 years ago

Yeah, I was having trouble with it. I might need other people's help to figure out what is going on. Basically when I try to use it using the BaseJoi.extend(JoiDateExtension) it is saying that BaseJoi isn't an instance of Joi so it throws an error. I'll upload the repo to github with a test and maybe we can figure out what is going on.

jeffbski commented 7 years ago

I have joi-date-extensions-browser repo up now and I have a buildable example which reproduces the error. If any of you would mind taking a look to see if you have any ideas how to resolve. https://github.com/jeffbski/joi-date-extensions-browser/issues/1

I also detail the error on the README for joi-date-extensions-browser as well with the exact steps to install and build.

jeffbski commented 7 years ago

@Marsup figured out that the webpack config was loading multiple instances of joi thus causing the problem in joi-date-extensions-browser/example/with-ext but I cannot determine what is wrong. In analyzing joi-browser and joi-date-extensions-browser they appear to be doing the right thing, but when I try to bring them together in a build, they are either pulling in both joi and joi-browser or two copies of joi-browser which in either case causes the extend to fail.

I'm out of ideas on how to change the webpack.config.js for the example or the package to fix this.

You can see https://github.com/jeffbski/joi-date-extensions-browser/issues/1 for all the details.

jeffbski commented 7 years ago

Since we still haven't figured out the issue with getting a build that can combine joi-browser and joi-date-extenstions-browser properly, I went ahead and created joi-full which is a universal package that will include the joi-date-extensions (and presumably other extensions in the future). It will work in node and in the browser (webpack and browserify).

Try it out and let me know if you have any problems.

I guess this will be the way to go until we get joi-date-extensions working together with joi-browser.

https://github.com/jeffbski/joi-full