Build CommonJS and UMD targets

[damusnet]

Here is my take on it. I removed webpack altogether in favor of Babel all the way. dist/strategy.js would now be the main target for all npm compatible bundlers. dist/strategy.min.js is the browser global export that people can include in their scripts tag in html. I also merged @jurassix commits to refactor src/strategy.js to full ES6 and upgrade the tests. I kept the sourcemap and added a watcher for dev puroposes.

ps: no need to update the docs, everything is the same, only it should work for everyone now :)

[jurassix]

@damusnet looks like we had similar ideas #3 :)

[damusnet]

@jurassix Absolutely! And since you beat me to the PR punch, I stole some code from you :)

[jurassix]

Nice - glad I could help. I'm going to close me PR then so @TheChech doesn't get overwhelmed.

[damusnet]

:+1:

[DanFMoore]

All works OK, but is there not a way to do this without having to compile strategy.js in order to run it in node? It just seems a bit of a faff.

Also if this is going to be merged in can you update the readme on how to use it? Since I've never used the export / import syntax you'd be better placed to do that.

[LKay]

:+1:

When it will get merge to the main branch I wonder? It will save a lot of trouble with workarounds to import it into browserified code.

Now even if it's app compiled by webpack/browserify by default browser version is being imported which ends up with en error module.exports = Validator. That happens obviously because Validator is expected to be globally defined variable which does not exist in the scope.

@TheChech : This is common practice recently for modules written in ES6 that you have src directory with pure ES6 code and lib or dist directory with transpiled code to ES5 which can be used in node or commonjs/amd. It's usually compiled during publishing to npm or just built manually every time before version upgrade.

ES6 import/export is not very different from node require/module.export - basic idea is the same. The only significant difference is asynchronous module load when using ES6 modules.

As long as during publishing both source and compiled versions will be present I don't see a reason to update readme as usage does not change at all. You can still include the file as <script> tag in the browser, require("react-validatorjs-strategy") for node or import strategy from "react-validatorjs-strategy" for ES6 modules.

[jurassix]

@damusnet @TheChech The README should be updated for ES5 users to include the dependency as: require("react-validatorjs-strategy").default (Note: this is introduced by Babel and can be tweeked by other plugins later if needed).

However, usage as a Node module is sufficient: npm i --save react-validatorjs-strategy. There are no peerDependencies or anything else that users need to know for consuming this library as a CommonJS dependency in ES6.

@LKay Thanks for your write up. I think for JS <= ES5 you need to use the following require style: require("react-validatorjs-strategy").default

[DanFMoore]

It does appear to be different; whereas before I could import it directly, I now have to access .default, both in node and in the browser.

Would it be easier if I just dropped all ES6 syntax and just used ES5?

[LKay]

@jurassix No you don't need to use .default after transpiling code with babel. Nevertheless I think you don't need to transpile code both using commonjs plugin and then umd plugin. The umd plugin only will handle all of the three way of using the library (browser, commonjs and amd). Just have one task which does babel ./src/strategy.js --presets es2015 --plugins transform-es2015-modules-umd -o ./dist/strategy.js && cat ./dist/strategy.js | uglifyjs -o ./dist/strategy.min.js. Than way you can import it either usinr require or ES6 import straight away. Are you using babel 6?

@TheChech Transpiling is not costly in that case and it's just one more automated step, so you shouldn't care too much. On the other hand most modern code (especially for react) is being written in ES6, so you can be up to date with current trends so I would encourage you to use ES6. But having the code in good-old ES5 is obviously fine too.

[LKay]

@jurassix As for tests, you changed source files to ES6 but tests are still written in ES5 so I guess you're referring to that. Why don't you change the test to ES6 as well? You should also clean up package.json file after you removed webpack/browserify etc.

Anyway I'll happily contribute and could fix these things up for you if you want. Just let me know. I'm currently in the process of changing validation strategy from Joi based to this one so I'll be using this code either way.

[jurassix]

@TheChech @LKay For babel 6 modules written in <= ES5 must use the .default, in Babel 5 and below this was not the case. There really is no issue if this is documented. In fact this is exactly how the react-validation-mixin works today. However, if you don't like there is a babel plugin to behave like Babel 5; I don't think it's necessary to make the change but it's an option.

@LKay The UMD wrapper seems to make sense in your explanation, and seems to fit @TheChech needs. And as far as the tests go, there is no reason not to rewrite the tests to be ES6, except we may need to transpile them first, etc and I was leaving that to the author to pursue.

[DanFMoore]

@jurassix with the latest version of react-validation-mixin I've just tried npm install and then in the node console:

require('react-validation-mixin')

And it doesn't have a .default property. But if I try that with react-validatorjs-strategy, it does.

Is there a way to make the strategy more consistent with the mixin?

[jurassix]

Yeah, I need to upgrade the mixin to use Babel 6 instead of 5. On Mon, Feb 22, 2016 at 7:23 PM Dan notifications@github.com wrote:

@jurassix https://github.com/jurassix with the latest version of react-validation-mixin I've just tried npm install and then in the node console:

require('react-validation-mixin')

And it doesn't have a .default property. But if I try that with react-validatorjs-strategy, it does.

Is there a way to make the strategy more consistent with the mixin?

— Reply to this email directly or view it on GitHub https://github.com/TheChech/react-validatorjs-strategy/pull/4#issuecomment-187449903 .

[jurassix]

I'll also look did the plugin that removes the need for the .default

[DanFMoore]

Well it's not a problem having .default if that's what the standard is. I just need to update the readme since it's a breaking change. Just to clarify the two ways to import it are:

require('react-validatorjs-strategy').default

import strategy from 'react-validator-strategy'

?

[jurassix]

This seems to be a good resource for this topic: http://stackoverflow.com/questions/33505992/babel-6-changes-how-it-exports-default

[LKay]

@jurassix You're right, I was thinking ES6 way, and yeah, they changed exports.default behavior in babel 6.

@TheChech You could just use this babel-plugin-add-module-exports plugin in addition to commonjs one and there will be no breaking change.

[LKay]

@jurassix @TheChech Regardless previous comments, I think we're going too crazy right now for a simple, ~80 lines of code wrapper. :)

The original problem was to provide universal way to make this consumable by npm compliant environment without need to explicitly import strategy-cjs.js file. So we should solve this first by transpiling to umd and use single file for both browser and npm. The matter if the code wil be written in ES5 or ES6 is of lesser importance at this point, I believe.

[DanFMoore]

OK, I think what I'm going to do is use most of this pull request, but redo the main script in ES5. As you say it's a very small file, it seems a bit overcomplicated given that. That will also avoid the .default issue.

[DanFMoore]

Refactored slightly; the compiled dist/strategy.js was exactly the same src/strategy.js, so I've stopped compiling that and left just UMD. Renamed src to lib and repointed package.json main to lib/strategy.js.

Simplifies it a bit.