Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[NoQA] fix: real startup metric #39070

Merged
merged 2 commits into from
Apr 2, 2024
Merged

[NoQA] fix: real startup metric #39070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5bb5850
fix: real startup metric
kirillzyusko Mar 27, 2024
71b12ac
fix: prettier
kirillzyusko Mar 28, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/Expensify.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import NavigationRoot from './libs/Navigation/NavigationRoot';
import NetworkConnection from './libs/NetworkConnection';
import PushNotification from './libs/Notification/PushNotification';
import './libs/Notification/PushNotification/subscribePushNotification';
import Performance from './libs/Performance';
import StartupTimer from './libs/StartupTimer';
// This lib needs to be imported, but it has nothing to export since all it contains is an Onyx connection
import './libs/UnreadIndicatorUpdater';
Expand Down Expand Up @@ -130,6 +131,7 @@ function Expensify({

const onSplashHide = useCallback(() => {
setIsSplashHidden(true);
Performance.markEnd(CONST.TIMING.SIDEBAR_LOADED);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On splash hide we are considering the sidebar loaded(ie. performance entry name SIDEBAR_LOADED), wondering if that is correct or name should be different? Asking bcoz earlier we did it in setSidebarLoaded.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kirillzyusko I think I don't need to run the e2e test for this PR change, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Pujan92 from one side - you are right, it's confusing that we use SIDEBAR_LOADED when splash screen gets hidden. But the start mark is added in BaseSidebar component (on mount).

So in my opinion it's better not to rename it 🙂

}, []);

useLayoutEffect(() => {
Expand Down
1 change: 0 additions & 1 deletion src/libs/actions/App.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,6 @@ function setSidebarLoaded() {
}

Onyx.set(ONYXKEYS.IS_SIDEBAR_LOADED, true);
Performance.markEnd(CONST.TIMING.SIDEBAR_LOADED);
Performance.markStart(CONST.TIMING.REPORT_INITIAL_RENDER);
}

Expand Down
Loading