search

paldepind / union-type

A small JavaScript library for defining and using union types.
MIT License
477 stars 28 forks source link

added environment conditions #17

Closed alexeygolev closed 9 years ago

alexeygolev commented 9 years ago

16

paldepind commented 9 years ago

I don't like the style changes you've made. To me they increase the line count and reduce the readability.

Otherwise I think this is a splendid idea. I wasn't aware that some compiles could figure out removing the code block.

But what about the browser build we're making with browserify -s unionType ./union-type.js -o dist/union-type.j? As far as I can see accessing process.env.NODE_ENV in it would return in a runtime error.

alexeygolev commented 9 years ago

@paldepind it is ugly indeed! I definitely need to think about a nicer way. I use webpack for bundling so browser standalone problem didn't occur to me:-/ What can be interesting is actually engineering the lib the way it can be split into two... so if you want you require both and have validations and if you don't you require only the core one. This way it's totally up to user how he wants to go about optimising the code. The user could conditionally require based on whatever she or he needs. Thoughts?

alexeygolev commented 9 years ago

Also we could do something like this and export the needed code based on env (so the style won't be hurt) and then use envify on build for browser

alexeygolev commented 9 years ago

@paldepind so I did a quick experiment here is a gist. I used the version union-type I proposed in #18 but it can be done with the current version. This way it's up to the library consumer to do the validations. It also opens up the possibility of passing custom validator function if need be.