feat(filter-metadata): Filtered google lib metadata #14

nutboltu · 2019-03-04T04:46:16Z

This PR excluded following metadata to reduce the bundle size.
It's unlikely to use these meta.

noInternationalDialling
fixedLine
uan
tollFree
premiumRate
sharedCost
voip
personalNumber
pager
voicemail

Also minified the dist/libphonenumber.js file

…dle size

superhit0

will be needing more time to review further.

I have given some initial thoughts

src/minify.js

scripts/build-dependencies.sh

.travis.yml

superhit0 · 2019-03-04T05:13:54Z

@nutboltu can you also remove the sub module python-gflags as well?

its not required for development

superhit0 · 2019-03-04T05:49:51Z

Note to me: Lets look at the diff of the our generated files & this https://github.com/googlei18n/libphonenumber/blob/master/javascript/i18n/phonenumbers/metadatalite.js

superhit0 · 2019-03-04T06:12:06Z

@nutboltu
there is a problem with removing these metadatas:: the isvalidnumber starts giving wrong answers, for example we removed fixed line, then I tested a fixed line number from India, it said invalid, even though it was valid.

The element in printed array shows the value of isValid

Local:

Live:

nutboltu · 2019-03-04T08:07:08Z

@superhit0 This is the starting point. I think we need to list down the number type and other metadata which we will remove from the validation.

I tried adding fixed line and it works fine for your given example. In a broader view. we can remove
uan, tollFree, premiumRate, sharedCost, voip, personalNumber, pager, voicemail and also leadingDigits.

superhit0 · 2019-03-04T17:23:01Z

Also, we need to strengthen the tests in the repo, before we merge this one, I will try adding tests with different types of phonenumber as well.

@nutboltu @patw0929 I would request you guys to do the same as well please.

nutboltu · 2019-03-12T22:27:49Z

@superhit0 I tested it in react-intl-tel-input. Now it only supports mobile and fixed line. Is it okay not to support rest?

superhit0 · 2019-03-13T03:48:56Z

@nutboltu whats the file size now?

nutboltu · 2019-03-13T12:36:15Z

@superhit0 Currect file size is ~179KB

nutboltu · 2019-03-15T10:06:51Z

downgrading node version in CI. jestjs/jest#8069

superhit0 · 2019-03-16T05:34:59Z

hmm for 40 KB should we remove logics for different types of phones?
@patw0929 @nutboltu what do you think?

nutboltu · 2019-03-16T05:42:48Z

If we also remove fixed line it will down to ~139KB. It upon us what we want to keep and what we don't.

superhit0 · 2019-03-16T06:03:01Z

Thats the thing right ?
we cannot remove fixed line as most of the offices have fixed line numbers

nutboltu · 2019-03-17T21:47:23Z

@patw0929 , @superhit0
I would like to explain the facts. Previously our react-intl-tel-input@5.0.1 had a bundle size of 72.7KB. But now after incorporating this libphonenumber-js-utils, the size increases to 286.9KB.

This fact is we are only using following functions from this library.

formatNumber
getExampleNumber
getExtension
getNumberType
isValidNumber

But there's no tree shaking in the bundle. Considering those functions, I would say we can keep

formatNumber
getExtension
getNumberType

I am not sure what's the benefits of keeping getExampleNumber. Giving an example provided by google may not necessary. And this may reduce the bundle size.

Now the main reason behind this huge bundle size is this isValidNumber function. I can say every developer validates the phone number in the backend as well and keeping this validation in the client side caused ~160KB size data. Still if we want to keep the validation, instead of using this google compiler I would suggest to build our own compiler using google metadata. (e.g: libphonenumber-js).

There's few possible solution I can say:

Build our own compiler using google metadata.
Use libphonenumber-js. react-phone-number-input is using this library.
Reduce the supported number type. Because very few people are using the following type of number.
- noInternationalDialling
- uan
- tollFree
- premiumRate
- sharedCost
- voip
- personalNumber
- pager
- voicemail

We can only support mobile and fixedLine for now which will reduce ~40 KB. It's really a down side when we load this component in a slow internet connection. We should consider it seriously.

nutboltu · 2019-10-20T11:06:02Z

@patw0929 any thoughts about this PR?

patw0929 · 2019-10-20T12:07:36Z

Sweet! 🙌 I agree with you that we should consider reducing the bundle size.

Just curious, what's the difference between the three solutions you listed above?
If we choose to use libphonenumber-js instead, is the approach also support mobile & fixed line only? And how big is the bundle size?
We need more information to make a decision.

nutboltu · 2019-10-21T23:38:32Z

@patw0929

For the 1st solution we need to build our own compiler using google metadata. In this case, we can keep updated with google metadata like libphonenumber-js. It's kind of rebuilding libphonenumber-js in our own way.
For the 2nd solution, it's a dependency to this package.
They only support mobile. see this line
https://github.com/catamphetamine/libphonenumber-js/blob/5678d38e64d314d75dad994f67fd833f347551eb/source/tools/generate.js#L35
Their bundle size is 27.9Kb (https://bundlephobia.com/result?p=libphonenumber-js@0.2.2)
3rd solution is this PR where we specify the supported number type.

patw0929 · 2019-10-22T14:39:12Z

Thank you for your detailed explanation! 👍

If we only want to support mobile, maybe we can just use libphonenumber-js, for convenience.

I personally think it is worthy to support mobile & fixed line.
So if you guys has same thought, then which solution is better for the maintenance? And pros & cons?
Or if we choose 3rd solution (this PR), what's next? Is it ready for merge?
Thanks!

nutboltu · 2019-10-23T02:59:50Z

@patw0929 This PR is to support only mobile and fixed line. Please review this so that we can merge

package.json

patw0929

Looks great to me!

nutboltu · 2019-10-26T21:40:47Z

@patw0929 , @superhit0 are we happy to merge this PR?

Can you please review again?

nutboltu · 2019-12-04T01:30:03Z

@patw0929 I am going to merge this PR.

feat(filter-metadata): Filtered google lib metadata to reduce the bun…

e053e1b

…dle size

nutboltu added the enhancement New feature or request label Mar 4, 2019

nutboltu self-assigned this Mar 4, 2019

fix(filter-metadata): move libphonenumber cloning from travis to script

dc8ef97

superhit0 previously requested changes Mar 4, 2019

View reviewed changes

src/minify.js Outdated Show resolved Hide resolved

scripts/build-dependencies.sh Outdated Show resolved Hide resolved

.travis.yml Show resolved Hide resolved

nutboltu added 2 commits March 4, 2019 19:30

fix(filter-metadata): add fixedline and remove leadingdigits

4c27159

fix(filter-metadata): Remove getValidationerror from libphonenumberjs

f75185a

nutboltu added 5 commits March 13, 2019 23:50

fix(filter-metadata): remove excluded metadata from tempalte

dd19c72

doc(filter-metadata): Update README

ee5a418

refactor(filter-metadata): add dependencies i package.json

275b0ec

fix(package): update package lock

5d8950f

fix(ci-failure): downgrade node version due to ci failure

b797317

patw0929 reviewed Oct 23, 2019

View reviewed changes

package.json Show resolved Hide resolved

nutboltu added 3 commits October 25, 2019 08:51

chore: update package-lock

2fee943

docs: update readme with install instructions

34ca538

chore: use openjdk8 in travis

2c4a62e

patw0929 approved these changes Oct 26, 2019

View reviewed changes

nutboltu merged commit cea625a into master Dec 4, 2019

nutboltu deleted the feature-filter-metadata branch December 4, 2019 01:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(filter-metadata): Filtered google lib metadata #14

feat(filter-metadata): Filtered google lib metadata #14

nutboltu commented Mar 4, 2019 •

edited

Loading

superhit0 left a comment

superhit0 commented Mar 4, 2019

superhit0 commented Mar 4, 2019 •

edited

Loading

superhit0 commented Mar 4, 2019 •

edited

Loading

nutboltu commented Mar 4, 2019

superhit0 commented Mar 4, 2019

nutboltu commented Mar 12, 2019

superhit0 commented Mar 13, 2019

nutboltu commented Mar 13, 2019

nutboltu commented Mar 15, 2019

superhit0 commented Mar 16, 2019

nutboltu commented Mar 16, 2019 •

edited

Loading

superhit0 commented Mar 16, 2019

nutboltu commented Mar 17, 2019 •

edited

Loading

nutboltu commented Oct 20, 2019

patw0929 commented Oct 20, 2019

nutboltu commented Oct 21, 2019 •

edited

Loading

patw0929 commented Oct 22, 2019

nutboltu commented Oct 23, 2019

patw0929 left a comment

nutboltu commented Oct 26, 2019

nutboltu commented Dec 4, 2019

feat(filter-metadata): Filtered google lib metadata #14

feat(filter-metadata): Filtered google lib metadata #14

Conversation

nutboltu commented Mar 4, 2019 • edited Loading

superhit0 left a comment

Choose a reason for hiding this comment

superhit0 commented Mar 4, 2019

superhit0 commented Mar 4, 2019 • edited Loading

superhit0 commented Mar 4, 2019 • edited Loading

nutboltu commented Mar 4, 2019

superhit0 commented Mar 4, 2019

nutboltu commented Mar 12, 2019

superhit0 commented Mar 13, 2019

nutboltu commented Mar 13, 2019

nutboltu commented Mar 15, 2019

superhit0 commented Mar 16, 2019

nutboltu commented Mar 16, 2019 • edited Loading

superhit0 commented Mar 16, 2019

nutboltu commented Mar 17, 2019 • edited Loading

nutboltu commented Oct 20, 2019

patw0929 commented Oct 20, 2019

nutboltu commented Oct 21, 2019 • edited Loading

patw0929 commented Oct 22, 2019

nutboltu commented Oct 23, 2019

patw0929 left a comment

Choose a reason for hiding this comment

nutboltu commented Oct 26, 2019

nutboltu commented Dec 4, 2019

nutboltu commented Mar 4, 2019 •

edited

Loading

superhit0 commented Mar 4, 2019 •

edited

Loading

superhit0 commented Mar 4, 2019 •

edited

Loading

nutboltu commented Mar 16, 2019 •

edited

Loading

nutboltu commented Mar 17, 2019 •

edited

Loading

nutboltu commented Oct 21, 2019 •

edited

Loading