Migrate to ES6 by forabi · Pull Request #496 · validatorjs/validator.js

forabi · 2016-02-11T19:50:20Z

This is a large PR with some opinionated changes, so I'm not sure if it would be accepted, but I'd try anyway :)

Deprecate non-string coercion (major version upgrade): it is difficult to implement this functionality when each function is used separately.
Rewrite validation functions as ES6 modules, so we can now require each function separately without having to include the whole library, this will be helpful to anyone using validator on the client side, and will make it possible in the future to use tree shaking (i.e. removing unused code) when Node supports ES6 modules.

~~Functions are available on the root folder for convenience~~ Functions are available in the lib/ dir:

// ES5
var isEmail = require('validator/lib/isEmail');

// ES6
import isEmail from 'validator/lib/isEmail';

// ES6 with tree shaking
import { isEmail } from 'validator';

Of course you can still include the entire library as usual:

var validator = require('validator');
validator.isEmail('email@domain.com'); // => true

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.

npm run clean # removes all builds
npm run clean:browser
npm run clean:node
npm run build
npm run build:browser
npm run build:node
npm run lint # runs ESLint
npm run test

* 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

chriso · 2016-02-12T02:31:50Z

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 ^4

forabi · 2016-02-12T06:31:13Z

Great!
I've added a new function assertString that is called by every validation function and currently throws an error if the input is not a string, toString is still available but it should be called manually. We could print the deprecation notice in assertString and throw the error, or even restore the coercion behavior by simply calling toString inside assertString.

forabi · 2016-02-12T11:03:09Z

Just wanted to point out that accessing individual functions using validator/<functionName> is not currently implemented, instead one has to use validator/lib/<functionName>. I'm trying to figure out the best way to do it without polluting the root dir with too many files. We may need to add tests for these files too.

tunnckoCore · 2016-02-12T13:52:51Z

I'm trying to figure out the best way to do it without polluting the root dir with too many files

Try export-files, set just one index.js in lib :)

* 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

chriso · 2016-02-13T00:41:30Z

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.

chriso · 2016-02-13T00:43:50Z

bower.json

-    "tests",
-    "Makefile",
-    "package.json"
+    "Makefile"


The Makefile no longer exists.

… build

forabi · 2016-02-16T17:19:11Z

@chriso Tests are linted now. I've also added:

isMobilePhone validation for ar-SY (Arabic (Syria))
isAlpha and isAlphanumeric validation for various Arabic locales.

* Note: Looks like ESLint 2 has a bug with the rule "prefer-const", so it is disabled for now

forabi · 2016-02-21T13:35:29Z

@chriso upgraded to ESLint 2 👍

chriso · 2016-02-21T20:16:42Z

👍 we'll need to merge in the changes from master in the past week and then I'll look at merging this PR.

forabi · 2016-02-21T20:25:39Z

Do you want me to do that?

chriso · 2016-02-21T20:27:01Z

If you don't mind. It won't let me merge otherwise.

forabi · 2016-02-21T20:54:23Z

@chriso Done!

chriso · 2016-02-21T21:47:03Z

Apart from the README changes, this is ready to merge ✨

Migrate to ES6

chriso · 2016-02-21T22:42:09Z

v5 will land shortly.

forabi · 2016-02-21T22:48:11Z

Awesome! 😄
Thank you!

chriso · 2016-02-21T23:03:30Z

5.0.0 has been published. Thanks for the PR!

…ce it doesn't automatically coerce non-strings to strings anymore: validatorjs/validator.js#496

ldong · 2016-11-01T23:44:27Z

I dont understand why it does not work for Ember by default? Something like:

import validator from 'bower_components/validator-js';

I had to do this

bower install validator-js --save-dev

Then inside ember-cli-build.js, add this line of code app.import('bower_components/validator-js/validator.js');

@chriso Thanks for the help.

…ce it doesn't automatically coerce non-strings to strings anymore: validatorjs/validator.js#496

forabi added 5 commits February 11, 2016 19:58

Replace jshint with eslint

61906f0

Remove tests from bower ignore (does not exist)

5dc5c24

ESLint: ignore builds and tests

0ebb3b4

Restore test for AMD

2487421

* 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

Restore deprecation message but do not coerce

95f853b

forabi added 4 commits February 12, 2016 17:55

ESLint: move ignore to .eslintignore

9b73512

ESLint: extends not needed in test dir

150667d

Update builds

d5d53fe

chriso reviewed Feb 13, 2016
View reviewed changes

bower.json Outdated

"tests",

"Makefile",

"package.json"

"Makefile"

Copy link
Copy Markdown

Collaborator

chriso Feb 13, 2016

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.

Readd comments to browser builds, do not join variables in unminified…

daa442a

… build

Add Arabic locales with tests for isAlpha and isAlphanumeric

95f355c

forabi added 5 commits February 16, 2016 19:21

Update README: Add new Arabic locales

e44ca88

Prefer template strings

f919358

Upgrade to ESLint 2, remove unnecessary deps

af94fb7

* Note: Looks like ESLint 2 has a bug with the rule "prefer-const", so it is disabled for now

Remove old ESLint config file

b1ff8bc

Fix ESLint config

bf16624

Rebase on 4.9.0

be9fb9c

Fix README.md

a33b64b

chriso added a commit that referenced this pull request Feb 21, 2016

Merge pull request #496 from forabi/modules

396f2be

Migrate to ES6

chriso merged commit 396f2be into validatorjs:master Feb 21, 2016

rustybailey added a commit to express-validator/express-validator that referenced this pull request Mar 13, 2016

Downgrade validator. An upgrade will require a major version bump sin…

0241908

…ce it doesn't automatically coerce non-strings to strings anymore: validatorjs/validator.js#496

rustybailey mentioned this pull request Mar 13, 2016

Outdated dependencies express-validator/express-validator#214

Closed

joerx mentioned this pull request Mar 27, 2016

deps: validator@5.1.0 TryGhost/Ghost#6645

Merged

3 tasks

rustybailey mentioned this pull request Apr 11, 2016

Fixing warning from node-validator express-validator/express-validator#226

Closed

kevinansfield mentioned this pull request Jun 1, 2016

deps: validator-js@5.3.0 TryGhost/Admin#34

Closed

dbettini mentioned this pull request Feb 9, 2018

Custom validation not documented completely sequelize/sequelize#8905

Closed

snyk-bot mentioned this pull request Jul 18, 2020

[Snyk] Fix for 17 vulnerabilities jahil-khalfe/freecodecamp#31

Open

snyk-bot mentioned this pull request Nov 13, 2020

[Snyk] Fix for 41 vulnerabilities Tshegofatso/freecodecamp#9

Merged

TigerStar429 pushed a commit to TigerStar429/expressjs-validator that referenced this pull request May 30, 2023

Downgrade validator. An upgrade will require a major version bump sin…

dd692ed

…ce it doesn't automatically coerce non-strings to strings anymore: validatorjs/validator.js#496

Uh oh!

Conversation

forabi commented Feb 11, 2016

Uh oh!

chriso commented Feb 12, 2016

Uh oh!

forabi commented Feb 12, 2016

Uh oh!

forabi commented Feb 12, 2016

Uh oh!

tunnckoCore commented Feb 12, 2016

Uh oh!

chriso commented Feb 13, 2016

Uh oh!

chriso Feb 13, 2016

Choose a reason for hiding this comment

Uh oh!

forabi commented Feb 16, 2016

Uh oh!

forabi commented Feb 21, 2016

Uh oh!

chriso commented Feb 21, 2016

Uh oh!

forabi commented Feb 21, 2016

Uh oh!

chriso commented Feb 21, 2016

Uh oh!

forabi commented Feb 21, 2016

Uh oh!

chriso commented Feb 21, 2016

Uh oh!

chriso commented Feb 21, 2016

Uh oh!

forabi commented Feb 21, 2016

Uh oh!

chriso commented Feb 21, 2016

Uh oh!

ldong commented Nov 1, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants