kentcdodds / webpack-validator-DEPRECATED

Use this to save yourself some time when working on a webpack configuration.
MIT License
93 stars 8 forks source link

Solve validation with Joi #59

Open jonathanglasmeyer opened 8 years ago

jonathanglasmeyer commented 8 years ago

So the more of those validate functions i'm writing the more i'm asking myself if this approach is optimal. We are in fact doing 95% schema validation, a task which a library like joi (or maybe the one you wrote, Kent, api-check ?) solves perfectly, but we are writing this very imperatively and with ad hoc, non-normalized error messages.

In fact i am thinking of experimentally starting an alternative approach in another repo, maybe just called webpack-joi-schema and see how far this would replicate the use case of our approach here. The main goal would be finding out where the limits of using a schema validation library would be and maybe check out if the burden of all those manual tests could be "extracted" onto the shoulders of a mature, more generic library -- thereby substantially reducing the error surface.

kentcdodds commented 8 years ago

Sure, knock yourself out. I decided to not use those exclusively because I felt like they lack the flexibility we want in creating useful error messages and checking things like whether files exist (which we're actually not doing anymore anyway).

jonathanglasmeyer commented 8 years ago

Error messages are a two-edged sword, we want them to be helpful and customizable, but on the other hand the majority of them are generic messages like "Must be an array" or "String must be one of the following", which would actually benefit from being normalized as this would increase polish. Additionally i believe that one can override generic messages in relevant cases. I also believe that custom validators are possible. We'll see, i'll play around with this for a bit. :)

kentcdodds commented 8 years ago

I should clarify, if you can get it working well then I'm totally on board :-)

xjamundx commented 8 years ago

Yeah super good idea On Sat, Mar 26, 2016 at 9:52 AM Kent C. Dodds notifications@github.com wrote:

I should clarify, if you can get it working well then I'm totally on board :-)

— You are receiving this because you are subscribed to this thread. Reply to this email directly or view it on GitHub https://github.com/kentcdodds/webpack-validator/issues/59#issuecomment-201891753

jonathanglasmeyer commented 8 years ago

A little update on my experiment. :)

Joi error formatting goodness

validation-1

Custom error messages are possible

validate-3

Declarative schema definition of module property

schema

So far this is really convincing.

You can have a look at the code at jonathanewerner/webpack-joi-schema.

I'm pretty happy with this. :)

kentcdodds commented 8 years ago

Looks really promising! You have the crazy syntax highlighting I've ever seen.

I'm pretty sure that this is where we wanna go. It's nice and declarative. And the output seems more helpful. The module property would be a beast to validate imparatively so I think this is cool.

One concern, the message:

"value" You can only pass the `query` property when you...

Is exactly the reason I started doing this with my own thing. What the heck does it mean by "value". I worry that the messaging isn't flexible enough or something. Thoughts?

jonathanglasmeyer commented 8 years ago

That's a vim plugin called semantic-highlight, it colors unique tokens in unique colors. :) I think it's the root of an inspected object is called value. So for the "production" use case of validating the whole webpack config and not isolated properties as shown in the screenshot this shouldn't occur at all, I think. You can also override the label if needed and specify its position in the message via inserting {{originalKeyName}} on the needed position.

So what does it mean for the general architecture of this project? I like webpack-validator much better as a name and also don't wanna fragment this. I think I've got about 80-90 percent of the webpack config api covered, it's just so much faster writing it like this. I've also abstracted test code quite a bit, maybe you wanna have a look at my repo. (Edit: i just saw that i forgot to push yesterday and now may laptop is at work. Well, the gist of it is visible in the test for module anyways). I sometimes like to write tests "data-driven", which I applied there.

Let's think how we can best integrate the efforts.

kentcdodds commented 8 years ago

Honestly, I'd be happy to abandon this project in favor of yours. I'd happily turn over the name if you'd like. We're still not technically even 1.0.0 yet so I could give you publish rights and you could just publish 1.0.0 when you feel like it's ready. Thoughts?

jonathanglasmeyer commented 8 years ago

Ok, lets do it like that. I'm impressed by your selflessness. :+1: So how does "giving publishing rights" work exactly with npm? Btw, I think I acted a bit hasty by publishing webpack-joi-schema to npm yesterday (it was my first npm package, yey:D) -- today I was a bit shocked to see it has apparently been downloaded 260 times since then, although I marked it as beta. What is the right way to do this? Rename the package? Deprecate it and forward to webpack-validator package? especially in light of the immutably announcements from npm today I'm a bit lost. :)

kentcdodds commented 8 years ago

Yeah, we definitely want to maintain immutability in npm (even though it's technically not enforced). So here's what I propose we do:

  1. I give you publish rights to webpack-validator. I can do this in the npm UI right.... now... done.
  2. You change the name in the package.json of webpack-joi-schema to webpack-validator and publish a beta of 1.0.0-beta.8
  3. You deprecate webpack-joi-schema with a message indicating that people should use webpack-validator.
  4. You can rename your github repo if you want. Make sure to update the relevant package.json fields.
  5. We work out what needs to happen to get a solid 1.0.0 for webpack-validator.
  6. Release webpack-validator 1.0.0 and celebrate! :tada:
  7. Figure out what needs to happen to support Webpack 2 (would be nice to support both).

I'm impressed by your selflessness. :+1:

Not sure what you mean. This is just as much your project as it is mine ;-)

Thanks for working hard to make this a thing!

jonathanglasmeyer commented 8 years ago

Ok, for my understanding:

Re selflessness: you're of course right -- I was just referring to the owner of the repo, but that's probably silly anyhow, should better be an github organization no? Or is that overkill? i kinda like that it visually reenforces the fact that the project doesn't have a single owner / maintainer.

kentcdodds commented 8 years ago

You use the deprecate command from the npm CLI. I don't have the docs link handy, but it should be easy to find. You should also update the README though.

It's up to you if you want an org. I'm fine either way :-)

jonathanglasmeyer commented 8 years ago

:+1: ok I'll do this tomorrow morning probably.

sarbbottam commented 8 years ago

This thread is such a good read!! Not only, useful information and tips, but a great example of constructive collaboration. :trophy:

jonathanglasmeyer commented 8 years ago

Done. :tada:

kentcdodds commented 8 years ago

Moving discussion over to your repo. Going to update the README here.