jackocnr / intl-tel-input

A JavaScript plugin for entering and validating international telephone numbers. React and Vue components also included.
https://intl-tel-input.com
MIT License
7.69k stars 1.95k forks source link

Allow `utilsScript` to be a function that returns a promise for the utils module #1816

Closed Mr0grog closed 1 month ago

Mr0grog commented 1 month ago

I might be missing the obvious, but it seems like a bit of a bear to lazy load the utilsScript with Webpack or some other bundler, since you need to figure out how to copy the script out of the intl-tel-input package and keep track of the path so it can be loaded. (Please let me know if there’s a simpler way that refers to a local copy and not a public CDN!)

It would be lovely if the utilsScript option could a function that returns a promise for the module. That way, you could have code like:

import intlTelInput from 'intl-tel-input';

const input = document.querySelector("#phone");
intlTelInput(input, {
    utilsScript: () => import('intl-tel-input/utils')
});

…since bundlers like Webpack can pick up on that dynamic import and split it out into a separate file automatically.

I can see that, right now, I could replace the intlTelInput.loadUtils() function, but that leaves me in charge of setting state-related stuff like intlTelInput.startedLoadingUtilsScript, which doesn’t seem like a public API I should expect to be stable.

Plugin version

v24.5.0

rashadmehtab commented 1 month ago

Here is correct method:

import intlTelInput from 'intl-tel-input/intlTelInputWithUtils';

const options={
  countryOrder: ["AE"],
  separateDialCode: true,
  lazyLoading: true,
  showFlags:true,
  showSelectedDialCode:true,
  placeholderNumberType: "MOBILE",
  formatAsYouType:false,
  strictMode:true
}
intlTelInput(input, options);
Mr0grog commented 1 month ago

@rashadmehtab that doesn't lazy load utils, though, does it?

Mr0grog commented 1 month ago

FWIW, and in case anybody else is trying to figure out how to lazy-load locally with Webpack, I think the simplest possible setup right now is:

// webpack.config.js

const config = {
  module: {
    rules: [
      {
        resourceQuery: /file-url/,
        type: 'asset/resource',
      },
      // ...other rules...
    ]
  },
  // ...other config...
}
// In your JS code

import intlTelInput from 'intl-tel-input';
import intlTelInputUtilsUrl from 'intl-tel-input/build/js/utils.js?file-url';

const input = document.querySelector("#phone");
intlTelInput(input, {
    utilsScript: intlTelInputUtilsUrl
});

But this only works in Webpack, and other bundlers would need a different setup (or couldn’t accomplish this on their own at all). It’s also still a little less nice because it requires configuration changes.

jackocnr commented 1 month ago

@Mr0grog I agree with your proposal! I think we should rename the utilsScript option to loadUtilsOnInit and rename the loadUtils static method to loadUtilsNow (or something like that), and both of them take a function which dynamically imports the utils script, as you suggest. This would just require a fairly minor change to the current loadUtils static method to replace the current hardcoded dynamic import with invoking the passed function instead, and keep the current .then() and .catch() code. What do you think?

Unfortunately I don't have time to implement this myself right now, but I would welcome a pull request.

One question though, re: your example code in your first post: utilsScript: () => import('intl-tel-input/utils') - are you sure webpack will understand this path - isn't it only resolved at runtime? Or does webpack do some magic and resolve it at build time?

Mr0grog commented 1 month ago

utilsScript: () => import('intl-tel-input/utils') - are you sure webpack will understand this path - isn't it only resolved at runtime? Or does webpack do some magic and resolve it at build time?

It does some magic at build time! (Same with Rollup/Vite, and I think also Parcel.) Whenever it sees a dynamic import() call in your code, Webpack tries to infer what module you are loading. Then it bundles that module (and its dependencies) as a separate file with all the right hashes and and so on in the file name, and replaces the dynamic import with the final output path/URL of that separate bundle file. Pretty fancy stuff.

Turning off the magic (so it doesn’t yell at you that it can’t figure out what you’re trying to import at build time) is what these comments in loadUtils() do: https://github.com/jackocnr/intl-tel-input/blob/3897106759372287b8ba74525cd9003a32d6c5c6/src/js/intl-tel-input.ts#L2111

I think we should rename the utilsScript option to loadUtilsOnInit and rename the loadUtils static method to loadUtilsNow (or something like that)

FWIW, I think loadUtils is already a good and pretty clear name for the static function — it does exactly what it says, and does it when you call it. For me, at least, adding “now” to the end doesn’t add much clarity, but I am happy to rename it however you’d prefer. :)

I ought to be able to throw together a PR for this, although it might be a few days. 👍


Total side note: I’ve noticed that errors loading the utils script cause some unhandled promise rejections to show up in the browser console:

Screenshot of errors in the browser console

I was thinking I might also try and make those both a little more graceful and informative as part of this. Does that seem good, or is it better to put that in a separate PR? (Or not at all?)

jackocnr commented 1 month ago

It does some magic at build time!

Great, so yes this change would be a big improvement (simplification) for those using bundlers.

FWIW, I think loadUtils is already a good and pretty clear name for the static function

Fine.

I ought to be able to throw together a PR for this, although it might be a few days.

Amazing 👍🏻

I’ve noticed that errors loading the utils script cause some unhandled promise rejections to show up in the browser console. I was thinking I might also try and make those both a little more graceful and informative as part of this.

That would be great, thanks.