fix: leverage tree-shakeable validator imports

[banjankri]

At the moment when using class-validator we are pulling in entire validator.js library due to the syntax used.

I would like to propose to migrate the the tree-shakeable imports instead referenced in validator.js README: https://github.com/validatorjs/validator.js#tree-shakeable-es-imports

[Kiliandeca]

Great idea, I would like to work on a PR for this

[Kiliandeca]

Hello, I made some tests and here's what I've got so far:

1. We probably need to wait from [Snyk] Upgrade @types/validator from 13.0.0 to 13.1.0 #664 to be merged since it added Type Definitions for the ES folder

2. I tried with the first validator

// We can't import with the same name since it's already used by the function below
import validatorContains from "validator/es/lib/contains" 

export function contains(value: unknown, seed: string): boolean {
    return typeof value === "string" && validatorContains(value, seed);
}

But when I tried to run the tests, Jest complain about how the validator es lib is importing internally

Jest encountered an unexpected token

    This usually means that you are trying to import a file which Jest cannot parse, e.g. it's not plain JavaScript.

    By default, if Jest sees a Babel config, it will use that to transform your files, ignoring "node_modules".

    Here's what you can do:
     • To have some of your "node_modules" files transformed, you can specify a custom "transformIgnorePatterns" in your config.
     • If you need a custom transformation specify a "transform" option in your config.
     • If you simply want to mock your non-JS modules (e.g. binary assets) you can stub them out with the "moduleNameMapper" config option.

    You'll find more details and examples of these config options in the docs:
    https://jestjs.io/docs/en/configuration.html

    Details:

    /<my-path>/class-validator/node_modules/validator/es/lib/contains.js:1
    import assertString from './util/assertString';
    ^^^^^^

    SyntaxError: Cannot use import statement outside a module

I already tried following the hint of using transformIgnorePatterns in the jest config file but it didn't work

   transformIgnorePatterns: [
       "/node_modules/(?!validator)"
   ],

I really don't know what to do next, maybe someone has more insight on the matter ?

note: It seem related to a problem someone already had with validatorjs validatorjs/validator.js#1208 (comment)

[banjankri]

It is correct, you need the newest typings to be merged in first, before you can proceed with using es/lib imports.

Have you maybe tried import someValidatorMethod as aliasOfThatMethod from '....' syntax to go around the clashes?

As for Jest, I would need to debug it to be able provide insight into what might be happening there.

[Kiliandeca]

You can't do import someValidatorMethod as aliasOfThatMethod from '....' it's not a valid syntax.

The lib expose the default export so from what I understand import aliasOfThaMethod from '...' already do the alias.

What we can do is:
import * as aliasOfThatMethod from "validator/es/lib/someValidator" or import { default as aliasOfThatMethod } from "validator/es/lib/someValidator"

But we get the same error: Cannot use import statement outside a module

[banjankri]

Would this give you some pointers? jestjs/jest#9292? You are seeing it when running units on Jest, right?

[Kiliandeca]

You are seeing it when running units on Jest, right?

Correct

I found the solution following jestjs/jest#9395 (comment)

To make it work I had to make these changes:

//package.json
{
  ...
  "devDependencies": {
    "@babel/preset-env": "^7.10.4",
  ...
  }

//jest.config.js
module.exports = {
    transform: {
        ...
        "^.+\\.js?$": "babel-jest" 
    },
    ...
    transformIgnorePatterns: [
        "node_modules/(?!validator)/"
    ]
};

//babel.config.json
{
  "presets": ["@babel/preset-env"]
}

Since it will add a dev dependencie and change the build process I would like to have the opinion of a maintainer before starting to work on the PR, I don't want to mess up the build process.

[Kiliandeca]

Seems like @NoNameProvided is already working on it d797c05

[NoNameProvided]

This is now in develop. Will be released in next version.

[janis91]

This'd be a huge performance improvement. Especially libphonenumer-js should be excluded, if no validator is used connected to it. @NoNameProvided do you plan to do that?

[NoNameProvided]

This is released.

[github-actions]

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.