search

patw0929 / react-intl-tel-input

Rewrite International Telephone Input in React.js. (Looking for maintainers, and PRs & contributors are also welcomed!)
https://patw0929.github.io/react-intl-tel-input/
MIT License
284 stars 220 forks source link

Remove the dependency on `window` #348

Open mcataford opened 4 years ago

mcataford commented 4 years ago

Phone number validation happens via utilities that are asynchronously loaded into window by libphonenumber-js-utils. This has been shown to cause problems in certain test runners (#347) and is generally a practice that should be avoided because it puts critical business logic in a place that the library has very little control over (the window global).

This issue exists to track refactoring work around not having that asynchronous dependency.

Expected Behavior

The library should not rely on window for critical functionality and should not asynchronously import packages.

Current Behavior

libphonenumber-js-utils is imported asynchronously for its side-effects, and critical logic is held in window.

Possible Solution

Refactor both react-intl-tel-input and libphonenumber-js-utils so that there is a synchronous path to import and use those utilities without relying on window as a communications channel.

Environment

mcataford commented 4 years ago

@patw0929 I was wondering if you had any thoughts on this.

From what I saw, the original libphonenumber implementation of the functions react-intl-tel-input uses hasn't really moved in two years, so I'd be tempted to sever the tie to the Closure Compiler, trim libphonenumber-js-utils down to a library that can be synchronously imported like any other and that doesn't put critical logic in window. :thinking:

We could still pull the same data, but have pure JS business logic and a build process that bundles it nicely. It would certain make the whole thing more accessible (libphonenumber-js-utils included) and would be a good step forward in optimizing react-intl-tel-input from a code complexity and general reliance on external resources point-of-view. I'd be more than happy to give that libphonenumber-js-utils work a shot, too.

BramKaashoek commented 4 years ago

I am experiencing segfaults in my CI/CD, probably because of this dynamic import as shown in https://github.com/nodejs/node/issues/27492.

patw0929 commented 4 years ago

@mcataford Nice idea!

I tried to make it easier to use & maintain (and keeping the functionality) before, but no luck (and no time). I am wondering if there is any other js libphonenumber project already make that & can work with react-intl-tel-input.

Again, thank you for the continued contributions! 🙇 (Invited you as libphonenumber-js-utils's collaborator too.)

mcataford commented 4 years ago

I quickly surveyed the landscape of libphonenumber ports, a lot of them seem to go the Closure Compiler route. Since libphonenumber-js-utils has some dependents, I think it's worthwhile to rework it either way, but I'll see if there's any likely candidates for react-intl-tel-input depending on work complexity! 🚀