-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Migrate to ES6 #496
Migrate to ES6 #496
Conversation
* Move each validation function to its own module, so you can now require only the functions you need e.g. require('validator/isEmail') instead of bundling the whole library in your scripts. * Use babel to build for node * Use rollup to generate an optimized browser bundle in umd format * Use npm scripts instead of make.
* define is a giant hack, looks like it will handle everything you throw at it * Here is the implementation https://github.com/jrburke/requirejs/blob/a1da39714bc3eea9ab85c77e47d544e815e2699f/require.js#L2049
Thanks for the PR! ✨ This will obviously take some time to review but I'm liking everything so far. I'm happy to move from jshint to eslint, and also happy to drop the makefile in favour of npm scripts. Dropping coercion of non-string input is already planned for v5, although I'd like for the deprecation notice to be out there in the wild for a bit longer, as there are undoubtedly projects and libraries that depend on the latest version of the library rather than |
Great! |
Just wanted to point out that accessing individual functions using |
Try |
* build no longer runs tests or cleans broken builds * add clean:browser and clean:node * add minify runs uglifyjs to generate validator.min.js * add pretest (alias to build) which is run automatically before tests * build-browser => build:browser * build-node => build:node * build:browser now eliminates dead code for unminified build
The point of the deprecation notice was that it still coerced input but warned when it did so, giving you time to update your code. Printing a deprecation notice and then throwing an error defeats the purpose. I'm happy for you to drop the deprecation notice entirely. Once this PR Is merged I'll tag and publish v5. |
"tests", | ||
"Makefile", | ||
"package.json" | ||
"Makefile" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Makefile no longer exists.
@chriso Tests are linted now. I've also added:
|
* Note: Looks like ESLint 2 has a bug with the rule "prefer-const", so it is disabled for now
@chriso upgraded to ESLint 2 👍 |
👍 we'll need to merge in the changes from master in the past week and then I'll look at merging this PR. |
Do you want me to do that? |
If you don't mind. It won't let me merge otherwise. |
@chriso Done! |
Apart from the README changes, this is ready to merge ✨ |
v5 will land shortly. |
Awesome! 😄 |
|
…ce it doesn't automatically coerce non-strings to strings anymore: validatorjs/validator.js#496
I dont understand why it does not work for
I had to do this
Then inside @chriso Thanks for the help. |
…ce it doesn't automatically coerce non-strings to strings anymore: validatorjs/validator.js#496
This is a large PR with some opinionated changes, so I'm not sure if it would be accepted, but I'd try anyway :)
Functions are available on the root folder for convenienceFunctions are available in thelib/
dir:Of course you can still include the entire library as usual:
Now to the opinionated changes:
Move to ESLint: it is extensible, has better support for ES6 and has awesome integration extensions for popular code editors.
Use npm scripts to build, clean and test the package as they are more compatible with non-*nix platforms and easier to use.