You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
[HOLD for payment 2024-10-16] [$250] [Performance] Avoid including mapbox in initial web bundle #48202
Reduce the amount of JS needed to load the app by delaying loading of resources that are not needed initially until they are used. One good candidate for this is mapbox, which is only used when creating a distance expense. The map is already not available while offline so this is not a concern.
What is the impact of this on end-users?
This app will load faster initially on web.
List any benchmarks that show the severity of the issue
We can see that mapbox is about 1mb (300kb gzip) of JS by running bundle analyzer (npm run analyze-packages). We can also see that this bundle is loaded initially by using chrome devtools network panel.
Proposed solution (if any)
Delay loading the map code until the component is rendered by using React.lazy. Offline mode is not a concern here since the map doesn't work while offline.
List any benchmarks after implementing the changes to show impacts of the proposed solution (if any)
We can see that mapbox is now in its own bundle by running npm run analyze-packages. We can also verify that this bundle is not loaded initially and only when rendering a screen that displays the map.
Platforms:
Which of our officially supported platforms is this issue occurring on?
Android: Native
Android: mWeb Chrome
iOS: Native
iOS: mWeb Safari
MacOS: Chrome / Safari
MacOS: Desktop
Version Number: 9.0.25-1 Reproducible in staging?: Yes Reproducible in production?: Yes Email or phone of affected tester (no customers): Logs:https://stackoverflow.com/c/expensify/questions/4856 Notes/Photos/Videos: Any additional supporting documentation Expensify/Expensify Issue URL: Issue reported by:@janicduplessis Slack conversation:
Auto-assigning issues to engineers is no longer supported. If you think this issue should receive engineering attention, please raise it in #whatsnext.
@neil-marcellini I know you worked with Mapbox. Do you mind taking over this issue and managing it? There is already a PR for it, I guess we'll want a C+ to review first.
Hi @janicduplessis. Thanks for working on this. I want to make sure this is a problem worth solving, as is our standard practice. Would you please present some benchmarks of the load time for the app with the mapbox bundle included and separate? Or otherwise determine what percentage of the load time comes from Mapbox with a direct measure? Please detail your testing methodology as well. Was a problem and solution presented anywhere in Slack? Would you please write one in the issue description?
Once I have that info I'll feel confident about going forward with this or not.
changed the title [-][Performance] Avoid including mapbox in initial web bundle[/-][+][$250] [Performance] Avoid including mapbox in initial web bundle[/+]on Sep 6, 2024
changed the title [-][$250] [Performance] Avoid including mapbox in initial web bundle[/-][+][HOLD for payment 2024-10-16] [$250] [Performance] Avoid including mapbox in initial web bundle[/+]on Oct 9, 2024
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.46-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:
Activity
melvin-bot commentedon Aug 28, 2024
Auto-assigning issues to engineers is no longer supported. If you think this issue should receive engineering attention, please raise it in #whatsnext.
cristipaval commentedon Aug 28, 2024
@neil-marcellini I know you worked with Mapbox. Do you mind taking over this issue and managing it? There is already a PR for it, I guess we'll want a C+ to review first.
melvin-bot commentedon Sep 3, 2024
Huh... This is 4 days overdue. Who can take care of this?
janicduplessis commentedon Sep 3, 2024
Waiting for review!
neil-marcellini commentedon Sep 4, 2024
@sobitneupane would you please review the PR as C+?
neil-marcellini commentedon Sep 5, 2024
Hi @janicduplessis. Thanks for working on this. I want to make sure this is a problem worth solving, as is our standard practice. Would you please present some benchmarks of the load time for the app with the mapbox bundle included and separate? Or otherwise determine what percentage of the load time comes from Mapbox with a direct measure? Please detail your testing methodology as well. Was a problem and solution presented anywhere in Slack? Would you please write one in the issue description?
Once I have that info I'll feel confident about going forward with this or not.
[-][Performance] Avoid including mapbox in initial web bundle[/-][+][$250] [Performance] Avoid including mapbox in initial web bundle[/+]melvin-bot commentedon Sep 6, 2024
Job added to Upwork: https://www.upwork.com/jobs/~021832118031291081965
25 remaining items
[-][$250] [Performance] Avoid including mapbox in initial web bundle[/-][+][HOLD for payment 2024-10-16] [$250] [Performance] Avoid including mapbox in initial web bundle[/+]melvin-bot commentedon Oct 9, 2024
The solution for this issue has been 🚀 deployed to production 🚀 in version 9.0.46-5 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:
If no regressions arise, payment will be issued on 2024-10-16. 🎊
For reference, here are some details about the assignees on this issue:
melvin-bot commentedon Oct 16, 2024
Payment Summary
Upwork Job
BugZero Checklist (@slafortune)
slafortune commentedon Oct 17, 2024
@janicduplessis does not require payment (Contractor)
@sobitneupane requires payment of $250 NewDot Manual Requests
garrettmknight commentedon Oct 23, 2024
$250 approved for @sobitneupane