Smaller bundle size #258

HonoluluHenk · 2018-09-13T01:00:27Z

I started on better modularization/build size reduction, references:

google-libphonenumber is huge fix: google-libphonenumber is huge #248
libphonenumber.js is loaded without using the IsMobilePhone decorator fix: libphonenumber.js is loaded without using the IsMobilePhone decorator #247
Smaller build size Smaller build size #212

This means moving most decorators and their respective validation implementation to their own file.

Since this is very tedious work: please review and tell me if this is the way to go.

Nice side-effect: also adds an easy way to validate with just a function (see ValidateBy.ts).

henrikra · 2018-09-16T19:30:00Z

test/functional/validation-functions-and-decorators.spec.ts

@@ -2864,7 +2863,7 @@ describe("isPhoneNumber", function() {
            someProperty: string;
        }

-        it("should not fail if validator.validate said that its valid", function(done) {
+        it.only("should not fail if validator.validate said that its valid", function(done) {


Should you remove only?

henrikra · 2018-09-20T13:55:19Z

@HonoluluHenk Are you still alive? :D

HonoluluHenk · 2018-09-23T09:29:17Z

Sorry, currently on holidays ☺️

HonoluluHenk · 2018-10-11T23:27:19Z

Since this introduces an API change (changed import) and requires lots of tedious work for me to implement this for all validator functions/annotations, I'd like to have the opinion of the package maintainer...

Care to comment, @NoNameProvided?

NoNameProvided · 2018-11-04T20:31:49Z

Hi @HonoluluHenk!

Yes, this is great work! I will help with the changes as my time allows. I have been inactive for a while now, but now I managed to spend time on OSS again (last week I have caught up with the changes in class-transformer, this week I will catch up with this lib. I will prioritize answers on issues first, but expect me to look/help with this on the weekend.)

henrikra · 2018-11-04T21:13:29Z

@NoNameProvided My pr would be easy to check out too :)

henrikra · 2018-11-05T05:52:43Z

@HonoluluHenk Can you fix conflicts?

HonoluluHenk · 2018-11-13T02:01:08Z

@NoNameProvided did some more migrations, but found a bug:
"context" is (imho) not implemented for custom validators.
I fixed this in 86079d8 but this commit needs proper review!

…Validations/validateValueByMetadata

HonoluluHenk · 2018-11-21T00:14:21Z

@NoNameProvided: migration done.
Most of the changed files are just the decorators and validation functions moved to their own file.
Files that require your special attention:
validation/ValidationExecutor.ts (I left one FIXME in validateOrReject)
validation/ValidationTypes.ts
validation/Validator.ts (validateValueByMetadata is gone since all metadata-validations are migrated to ValidateBy())
validation-schema/ValidationSchemaToMetadataTransformer.ts

HonoluluHenk · 2018-11-21T00:34:19Z

Measures with a minimal angular application:
Empty angular project without class-validator: 312k

Using class-validator 0.9.1: bundle total 864k

Compare to this refactoring:
Bundle size when adding class-validator: 328k (just using validate() and some simple type checkers)
Including @IsPhoneNumber: 772k (this is still less than v 0.9.1!)

henrikra · 2018-11-21T06:48:05Z

Wow I really like this. How do you measure bundle size btw?

Djaler · 2019-09-25T16:53:29Z

@maxime1992 in angular cli one can not change webpack config? In vue cli we can do it easily

maxime1992 · 2019-09-26T08:55:32Z

@maxime1992 in angular cli one can not change webpack config? In vue cli we can do it easily

You could eject or use ngx-build-plus for ex but it's definitely not worth it in this case as I do not use the library at all and a fix is coming. So I decided to go for the ugly quick fix there :)

tmtron · 2020-01-02T08:26:31Z

To get rid of libphonenumber for the angular-cli I came up with a hack. The idea is a combination of the comments from maxime1992 and Djaler

In your workspace save this libphonenumber-stub.js file:

module.exports = {
    PhoneNumberUtil: {
        getInstance() {
            return null;
        },
    },
};

in package.json use the postinstall hook to replace the original lib:

{
  "scripts": {
    "postinstall": "npx nodecat libphonenumber-stub.js > ./node_modules/google-libphonenumber/dist/libphonenumber.js || true"
   }
}

hint: we use npx nodecat (instead of plain cat) so that this also works on Windows

gempain · 2020-02-25T09:16:36Z

@NoNameProvided any chance this PR would be merged in ?

patrykkrawczyk · 2020-03-28T22:46:59Z

please merge this @vlapo @NoNameProvided @quentinus95

gempain · 2020-03-29T00:35:06Z

@HonoluluHenk would you be able to rebase your changes on the latest target branch so the conflicts are resolved ?

vlapo · 2020-03-30T15:24:16Z

🚀 I would like to ask you all for help in testing and reviewing PR #568 🚀

vlapo · 2020-04-03T07:43:41Z

@henrikra @gempain @patrykkrawczyk @maxime1992 @Djaler @ramyothman Could you please test preview release (#568) and provide some feedback?

gempain · 2020-04-03T10:24:01Z

@vlapo yes, I'll give it a shot. Actually, had a chance yesterday to try 12.0.0.rc-3 with jest and I had a few issues, but it was not a clean install so I need to see if I can reproduce on a clean install because I am wondering whether the problem was jest. Give me a day or two so I can do this. Thanks for the time you're putting into this, very appreciated.

gempain · 2020-04-04T11:23:15Z

@vlapo I've put together a demo repo, and so far I can't get tree shaking to work with 0.12.0.refactor-4. I followed webpack's official tree shaking guide which tells you to set "sideEffects": false" in package.json and optimization.usedExports to true in webpack.config.js, but I'd be greatful if someone can take a look at the repo and see if everything seems configured properly.

@vlapo feel free to use my repo to test out your changes or fork it. Do you want me to add you as a contributor so you can commit there ?

patrykkrawczyk · 2020-04-04T18:29:54Z

it's also not tree-shakebale in my project
without class-validator: 4 kb
with class-validator: 903 kb
this is using 0.12.0.refactor-4 and only doing import of IsString decorator

vlapo · 2020-04-04T20:36:07Z

Thank you for testing! 0.12.0-refactor.5 is published.
@gempain I test it on your demo app. But I think you have to update tsconfig and add:

"module": "es2015",
"moduleResolution": "node",

@patrykkrawczyk could you test 0.12.0-refactor.5? What is your environment? Do you use webpack in your project?

gempain · 2020-04-05T11:04:20Z

@vlapo after updating my tsconfig and upgrading to reafctor 5, it is now tree shakable ! Great job !

patrykkrawczyk · 2020-04-05T11:43:27Z

0.12.0-refactor.5 is tree shakeable in my project.
I'm writing AWS Lambda function using Serverless Framework, Node 12 and TS.
Without class-validator: 4.88KiB
With class-validator isString: 20.8 KiB
With class-validator @IsString: 21.6 KiB
With class-validator @IsPhoneNumber('DE'): 661 KB

gempain · 2020-04-05T19:33:06Z

@vlapo I think after this, we'll have to refactor the @IsPhoneNumber validator. I can't believe we're embedding 400kb just for validating a phone number. We'd need to drop libphonenumber.js.

vlapo · 2020-04-07T20:28:45Z

@gempain this PR is about keeping libphonenumber.js. After #568 PR merged we can use any lib (even a large one) for validation as tree shaking will remove unused code.

gempain · 2020-04-07T20:50:41Z

@vlapo sure, now people that don't use the phone validator won't get those 400kb, but people that do use it will get those kb, and I find it ackward that the validation logic of the phone validator weighs as much as the entire library. Anyway, that's another matter.

vlapo · 2020-04-07T20:57:36Z

Of course. We have to inform users about this disadvantage (as a note in docs). Developers can use validatorjs IsMobilePhone validator as alternative. IsPhoneNumber is some kind of heavy gun 🔫

But if someone know replacement for libphonenumber.js I am open for suggestions 🚀

BREAKING CHANGE: Validation functions was removed from `Validator` class to enable tree shaking. BEFORE: import {Validator} from "class-validator"; const validator = new Validator(); validator.isNotIn(value, possibleValues); validator.isBoolean(value); AFTER: import {isNotIn, isBoolean} from "class-validator"; isNotIn(value, possibleValues); isBoolean(value); Closes #258, #248, #247, #212

desmap · 2020-07-08T07:30:43Z

Pls check a part of my bundle graph:

Is this normal? I've import { IsMongoId, MaxLength, ArrayMaxSize } from 'class-validator and import { validate } from 'class-validator in my codebase.

My tsconfig matches the one from @gempain and my webpack conf is the one from Next + usedExports: true and I added "sideEffects": false" in package.json.

FWIW, this is a Next production build of the client side. I have v0.12.2 of class-validator.

ramyothman · 2020-07-10T16:52:02Z

Class validator uses the phone validator lib from google which makes its bundle big unfortunately

…

On Wed, Jul 8, 2020, 09:30 desmap ***@***.***> wrote: Pls check my part of my bundle graph: [image: image] <https://user-images.githubusercontent.com/43666255/86890019-f69d0a80-c0fc-11ea-9927-3d2f1908a230.png> Is this normal? I've import { IsMongoId, MaxLength, ArrayMaxSize } from 'class-validator and import { validate } from 'class-validator in my codebase. My tsconfig matches the one from @gempain <https://github.com/gempain> and my webpack conf is the one from Next + usedExports: true and I added "sideEffects": false" in package.json. FWIW, his is a Next production build of the client side. I have v0.12.2 of class-validator. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#258 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAU6KBHIBSWHWGU5M45CV43R2QODFANCNFSM4FUZS6PA> .

jbjhjm · 2020-07-13T08:45:31Z

@ramyothman should not be the case in most recent versions anymore (treeshakable validators). As you can see in @desmap 's chart, libphonenumber is not in there. I guess he/she is wondering why so many validators are bundled as just 3 were imported.

NoNameProvided · 2020-08-03T07:04:46Z

Just FYI, the package used for validating phone numbers has been replaced in the current develop branch with a package using the same meta information to validate the phone number but much smaller.

github-actions · 2020-09-03T00:28:24Z

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

henrikra reviewed Sep 16, 2018

View reviewed changes

HonoluluHenk force-pushed the feature/modularize branch from 2712625 to 108f560 Compare October 11, 2018 23:29

NoNameProvided self-assigned this Nov 4, 2018

NoNameProvided added the priority: high label Nov 4, 2018

This was referenced Nov 4, 2018

fix: libphonenumber.js is loaded without using the IsMobilePhone decorator #247

Closed

fix: google-libphonenumber is huge #248

Closed

Smaller build size #212

Closed

Christoph Linder added 6 commits November 13, 2018 00:02

Smaller bundle - kickoff

7664dd1

Smaller bundle - minor fix for preliminary unit test

f63fe69

Smaller bundle - next bunch of migrations...

7df6a60

Smaller bundle - remove global exports/fix imports

c734897

API breaking changes require major version bump => version 1.0.0-rc

2468930

Smaller bundle - rebase onto origin/master, fix conflicts

1320d29

HonoluluHenk force-pushed the feature/modularize branch from 5cc9b4d to 1320d29 Compare November 12, 2018 23:26

Christoph Linder added 2 commits November 13, 2018 02:57

Smaller bundle - next bunch of migrations...

3907863

fix "context" for custom validators

86079d8

Christoph Linder added 4 commits November 17, 2018 01:13

Smaller bundle - next bunch of migrations...

61130a5

Smaller bundle - undo accidental dependency downgrades

b8ee2b6

Smaller bundle - next bunch of migrations...

44e6227

Smaller bundle - last bunch of migrations, removed now unused default…

dec640d

…Validations/validateValueByMetadata

AndrinGautschi mentioned this pull request Feb 20, 2020

question: excluding google-libphonenumber #472

Closed

Larsrdev mentioned this pull request Feb 24, 2020

fix: split validators.ts into multiple files to allow proper tree shaking #519

Closed

vlapo mentioned this pull request Mar 30, 2020

refactor: update build process to enable tree shaking #568

Merged

vlapo closed this in #568 Apr 17, 2020

C0Nd3Mnd mentioned this pull request Jul 23, 2020

Support tree shaking (google-libphonenumber is huge) MichalLytek/class-transformer-validator#30

Open

github-actions bot locked as resolved and limited conversation to collaborators Sep 3, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Smaller bundle size #258

Smaller bundle size #258

HonoluluHenk commented Sep 13, 2018

henrikra Sep 16, 2018

henrikra commented Sep 20, 2018

HonoluluHenk commented Sep 23, 2018

HonoluluHenk commented Oct 11, 2018

NoNameProvided commented Nov 4, 2018

henrikra commented Nov 4, 2018

henrikra commented Nov 5, 2018

HonoluluHenk commented Nov 13, 2018

HonoluluHenk commented Nov 21, 2018

HonoluluHenk commented Nov 21, 2018

henrikra commented Nov 21, 2018

Djaler commented Sep 25, 2019

maxime1992 commented Sep 26, 2019

tmtron commented Jan 2, 2020 •

edited

Loading

gempain commented Feb 25, 2020

patrykkrawczyk commented Mar 28, 2020

gempain commented Mar 29, 2020

vlapo commented Mar 30, 2020

vlapo commented Apr 3, 2020

gempain commented Apr 3, 2020 •

edited

Loading

gempain commented Apr 4, 2020

patrykkrawczyk commented Apr 4, 2020

vlapo commented Apr 4, 2020

gempain commented Apr 5, 2020

patrykkrawczyk commented Apr 5, 2020

gempain commented Apr 5, 2020

vlapo commented Apr 7, 2020

gempain commented Apr 7, 2020

vlapo commented Apr 7, 2020

desmap commented Jul 8, 2020 •

edited

Loading

ramyothman commented Jul 10, 2020 via email

jbjhjm commented Jul 13, 2020 •

edited

Loading

NoNameProvided commented Aug 3, 2020

github-actions bot commented Sep 3, 2020

Smaller bundle size #258

Smaller bundle size #258

Conversation

HonoluluHenk commented Sep 13, 2018

henrikra Sep 16, 2018

Choose a reason for hiding this comment

henrikra commented Sep 20, 2018

HonoluluHenk commented Sep 23, 2018

HonoluluHenk commented Oct 11, 2018

NoNameProvided commented Nov 4, 2018

henrikra commented Nov 4, 2018

henrikra commented Nov 5, 2018

HonoluluHenk commented Nov 13, 2018

HonoluluHenk commented Nov 21, 2018

HonoluluHenk commented Nov 21, 2018

henrikra commented Nov 21, 2018

Djaler commented Sep 25, 2019

maxime1992 commented Sep 26, 2019

tmtron commented Jan 2, 2020 • edited Loading

gempain commented Feb 25, 2020

patrykkrawczyk commented Mar 28, 2020

gempain commented Mar 29, 2020

vlapo commented Mar 30, 2020

vlapo commented Apr 3, 2020

gempain commented Apr 3, 2020 • edited Loading

gempain commented Apr 4, 2020

patrykkrawczyk commented Apr 4, 2020

vlapo commented Apr 4, 2020

gempain commented Apr 5, 2020

patrykkrawczyk commented Apr 5, 2020

gempain commented Apr 5, 2020

vlapo commented Apr 7, 2020

gempain commented Apr 7, 2020

vlapo commented Apr 7, 2020

desmap commented Jul 8, 2020 • edited Loading

ramyothman commented Jul 10, 2020 via email

jbjhjm commented Jul 13, 2020 • edited Loading

NoNameProvided commented Aug 3, 2020

github-actions bot commented Sep 3, 2020

tmtron commented Jan 2, 2020 •

edited

Loading

gempain commented Apr 3, 2020 •

edited

Loading

desmap commented Jul 8, 2020 •

edited

Loading

jbjhjm commented Jul 13, 2020 •

edited

Loading