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

joi 10 #14

Open
k-sheth opened this issue Nov 24, 2016 · 12 comments
Open

joi 10 #14

k-sheth opened this issue Nov 24, 2016 · 12 comments

Comments

@k-sheth
Copy link

k-sheth commented Nov 24, 2016

Hi,

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

Thanks,

@jeffbski
Copy link
Owner

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

@jaredhirsch
Copy link

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

@k-sheth
Copy link
Author

k-sheth commented Nov 30, 2016

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
Copy link
Owner

@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
Copy link
Owner

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
Copy link
Author

k-sheth commented Nov 30, 2016

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

@jaredhirsch
Copy link

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

@k-sheth
Copy link
Author

k-sheth commented Dec 8, 2016

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

@jeffbski
Copy link
Owner

jeffbski commented Dec 8, 2016

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
Copy link
Owner

jeffbski commented Dec 9, 2016

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. jeffbski/joi-date-extensions-browser#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
Copy link
Owner

@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 jeffbski/joi-date-extensions-browser#1 for all the details.

@jeffbski
Copy link
Owner

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

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

No branches or pull requests

3 participants