API prep for reconnection callback syncing indicator #2524
Conversation
undo changes nully remove more unneeded changes
30767e6 to
8e308c9
Compare
|
Fixed conflicts on this one. |
| */ | ||
| function fetch() { | ||
| API.Get({ | ||
| function fetchPersonalDetails() { |
There was a problem hiding this comment.
NAB: Why did you rename fetch? Since it's under PersonalDetails, it implies it's personal details what it's retrieving.
There was a problem hiding this comment.
I guess this is a personal preference to put in my personal details ? 😖
When we have a generic export like fetch() it encourages renaming imports and ultimately makes it hard to track down usages of things e.g. take several examples:
Here we can rename this to whatever and it makes finding things hell
import {fetch as fetchDetails} from './PersonalDetails';import {fetch as fetchThePersonalDetails} from './PersonalDetails';import * as PersonalDetails;
PersonalDetails.fetch();Here there is no confusion and if we search for the name we'll always find and it's less likely someone will change the name to avoid a conflict with another generic fetch().
import * as PersonalDetails from './PersonalDetails';
PersonalDetails.fetchPersonalDetails();import {fetchPersonalDetails} from './PersonalDetails';Searching for fetchPersonalDetails in the latest case would bring up all usages.
There was a problem hiding this comment.
Gotcha, it now makes sense!
|
✋ This PR was not deployed to staging yet because QA is ongoing. It will be automatically deployed to staging after the next production release. |
|
🚀 Deployed to staging in version: 1.0.36-1🚀
|
|
🚀 Deployed to production in version: 1.0.39-5🚀
|
Details
Doing some prep work for E/E 159767
Fixed Issues (prep work does not fix)
https://github.com/Expensify/Expensify/issues/159767
Tests
isLoadingAfterReconnectisLoadingAfterReconnectis set totruethen eventually changes tofalse2021-04-23_09-17-26.mp4
QA Steps
Tested On
Screenshots
Web
Mobile Web
Desktop
iOS
Android