#4516: Move formatPhoneNumber (and libphonenumber-js) server-side to reduce client bundle #4517
Conversation
|
I don't think we should remove this. This is functionality we should provide. Would be nice if it could be dynamically imported though |
|
@zenweasel Thanks, I've updated this to use a dynamic import instead of removing it altogether. @ajporlante @Akarshit - this is ready for review. |
package.json
Outdated
| "jquery-i18next": "^1.2.1", | ||
| "later": "^1.2.0", | ||
| "libphonenumber-js": "^1.0.24", | ||
| "libphonenumber-js": "1.0.24", |
There was a problem hiding this comment.
The current version of this is 1.4.2, I wonder if we shouldn't update this now
There was a problem hiding this comment.
@zenweasel Updated libphonenumber-js and re-tested.
|
@dancastellon @zenweasel As far as I can tell, only SMS pkg is using this, and it uses it from server code. Going forward, the preferred way for clients to get a formatted phone number would be for it to be made available through GraphQL (e.g., a So given these facts, how about we move this function to If we want to keep SMS pkg working, you could update that one line that imports it in that repo. |
…reaction into perf-4516-remove-libphonenumber-js
|
Thanks @aldeed, PR updated |
Resolves #4516
Impact: minor
Type: performance
Issue
libphonenumber-js was being included in the client bundle. It's only use is in
formatPhoneNumber.Solution
Turn
formatPhoneNumberinto an async function that dynamically imports libphonenumber-js. In a separate PR, update the plugin that uses it (SMS in Reaction Contrib repo)Breaking changes
None
Testing
import { formatPhoneNumber } from "/lib/api/helpers";
async function formatTest() {
const num = await formatPhoneNumber("5555555555", "US");
console.log("Formated phone number:", num);
}
formatTest();
export TOOL_NODE_FLAGS='--max-old-space-size=4096' && METEOR_DISABLE_OPTIMISTIC_CACHING=1 meteor run --production --extra-packages bundle-visualizerBefore
After