fix: Refactor faster page loads (#25983)

## Description
Improve page loads by removing the blocking calls of user and tenant
information. These calls were called before any other routes would load
and that would increase the time taken for other subsequent calls.

Reasons it was needed for 
1. Branding info comes from tenant api call. This being executed later
would cause flashing of colours if branding is updated
2. Licence info comes from tenant api call. A check is needed for
licence validity and show appropriate screens instead of the app for
users
3. User info was needed to show the correct admin settings to the user

We are going to mitigate dependency 1 and 2 by hiding the ui by making
the opacity to 0 till the responses are not received. This will ensure
users don't see the screen, yet the JS is being loaded efficiently and
api calls are done earlier.
We will move the admin settings dependency to the route itself so that
other routes do not delay because of this change

#### PR fixes following issue(s)
Fixes #25586

#### Media
> A video or a GIF is preferred. when using Loom, don’t embed because it
looks like it’s a GIF. instead, just link to the video
>
>
#### Type of change

- Chore (housekeeping or task changes that don't impact user perception)

## Testing

#### How Has This Been Tested?
Needs regression testing on EE with admin settings, branding and licence
checks
Page loads on CE and EE

- [x] Manual
- [ ] Cypress

#### Test Plan
EE:
- [ ] admin settings
- [ ] branding
- [ ] licence checks
- [ ] Page loads

CE
- [x] admin settings
- [x] Page loads
>
#### Issues raised during DP testing
No issues found during DP (CE) Testing

## Checklist:
#### Dev activity
- [ ] My code follows the style guidelines of this project
- [ ] I have performed a self-review of my own code
- [ ] I have commented my code, particularly in hard-to-understand areas
- [ ] I have made corresponding changes to the documentation
- [ ] My changes generate no new warnings
- [ ] I have added tests that prove my fix is effective or that my
feature works
- [ ] New and existing unit tests pass locally with my changes
- [ ] PR is being merged under a feature flag


#### QA activity:
- [ ] [Speedbreak
features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-)
have been covered
- [ ] Test plan covers all impacted features and [areas of
interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-)
- [ ] Test plan has been peer reviewed by project stakeholders and other
QA members
- [ ] Manually tested functionality on DP
- [ ] We had an implementation alignment call with stakeholders post QA
Round 2
- [ ] Cypress test cases have been added and approved by SDET/manual QA
- [ ] Added `Test Plan Approved` label after Cypress tests were reviewed
- [ ] Added `Test Plan Approved` label after JUnit tests were reviewed
This commit is contained in:
Hetu Nandu 2023-08-11 13:30:46 +05:30 committed by GitHub
parent 252d2ecddf
commit db82cca253
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 84 additions and 146 deletions

View File

@ -14,6 +14,9 @@
background: #D7D7D7;
transition: all ease-in 0.3s;
}
#app-load-block {
visibility: hidden;
}
</style>
<script>
// '' (empty strings), 'false' are falsy
@ -91,8 +94,10 @@
To keep zIndex for tooltips higher than app comments, todo remove when migrating to Tooltip2
Currently the className does not apply to the portal root, so we're unable to work with z-indexes based on that
-->
<div id="header-root"></div>
<div id="root"></div>
<div id="app-load-block">
<div id="header-root"></div>
<div id="root"></div>
</div>
<div id="date-picker-control" style="position: relative; z-index: 1000;"></div>
<script type="text/javascript">
// Ref: https://github.com/Modernizr/Modernizr/blob/94592f279a410436530c7c06acc42a6e90c20150/feature-detects/storage/localstorage.js
@ -211,4 +216,4 @@
</script>
</body>
</html>
</html>

View File

@ -2,12 +2,6 @@ import type { APP_MODE } from "entities/App";
import type { ReduxAction } from "@appsmith/constants/ReduxActionConstants";
import { ReduxActionTypes } from "@appsmith/constants/ReduxActionConstants";
export const initCurrentPage = () => {
return {
type: ReduxActionTypes.INITIALIZE_CURRENT_PAGE,
};
};
export type InitializeEditorPayload = {
applicationId?: string;
pageId?: string;

View File

@ -1,4 +1,4 @@
import React, { Suspense, useEffect } from "react";
import React, { Suspense } from "react";
import history from "utils/history";
import AppHeader from "pages/common/AppHeader";
import { Redirect, Route, Router, Switch } from "react-router-dom";
@ -38,35 +38,23 @@ import ErrorPage from "pages/common/ErrorPage";
import PageNotFound from "pages/common/ErrorPages/PageNotFound";
import PageLoadingBar from "pages/common/PageLoadingBar";
import ErrorPageHeader from "pages/common/ErrorPageHeader";
import type { AppState } from "@appsmith/reducers";
import { connect, useSelector } from "react-redux";
import { useSelector } from "react-redux";
import * as Sentry from "@sentry/react";
import { getSafeCrash, getSafeCrashCode } from "selectors/errorSelectors";
import UserProfile from "pages/UserProfile";
import { getCurrentUser } from "actions/authActions";
import { getCurrentUserLoading } from "selectors/usersSelectors";
import Setup from "pages/setup";
import SettingsLoader from "pages/Settings/loader";
import SignupSuccess from "pages/setup/SignupSuccess";
import type { ERROR_CODES } from "@appsmith/constants/ApiConstants";
import TemplatesListLoader from "pages/Templates/loader";
import {
fetchFeatureFlagsInit,
fetchProductAlertInit,
} from "actions/userActions";
import { getCurrentTenant } from "@appsmith/actions/tenantActions";
import { getDefaultAdminSettingsPath } from "@appsmith/utils/adminSettingsHelpers";
import { getCurrentUser as getCurrentUserSelector } from "selectors/usersSelectors";
import {
getTenantPermissions,
isTenantLoading,
} from "@appsmith/selectors/tenantSelectors";
import useBrandingTheme from "utils/hooks/useBrandingTheme";
import { getTenantPermissions } from "@appsmith/selectors/tenantSelectors";
import RouteChangeListener from "RouteChangeListener";
import { initCurrentPage } from "../actions/initActions";
import Walkthrough from "components/featureWalkthrough";
import ProductAlertBanner from "components/editorComponents/ProductAlertBanner";
import { useFirstRouteLoad } from "../pages/loaderHooks";
export const SentryRoute = Sentry.withSentryRouting(Route);
@ -75,7 +63,6 @@ export const loadingIndicator = <PageLoadingBar />;
export function Routes() {
const user = useSelector(getCurrentUserSelector);
const tenantPermissions = useSelector(getTenantPermissions);
return (
<Switch>
<SentryRoute component={LandingScreen} exact path={BASE_URL} />
@ -131,60 +118,18 @@ export function Routes() {
);
}
function AppRouter(props: {
safeCrash: boolean;
getCurrentUser: () => void;
getFeatureFlags: () => void;
getCurrentTenant: () => void;
initCurrentPage: () => void;
fetchProductAlert: () => void;
safeCrashCode?: ERROR_CODES;
}) {
const {
fetchProductAlert,
getCurrentTenant,
getCurrentUser,
getFeatureFlags,
initCurrentPage,
} = props;
const tenantIsLoading = useSelector(isTenantLoading);
const currentUserIsLoading = useSelector(getCurrentUserLoading);
useEffect(() => {
getCurrentUser();
getFeatureFlags();
getCurrentTenant();
initCurrentPage();
fetchProductAlert();
}, []);
useBrandingTheme();
// hide the top loader once the tenant is loaded
useEffect(() => {
if (tenantIsLoading === false && currentUserIsLoading === false) {
const loader = document.getElementById("loader") as HTMLDivElement;
if (loader) {
loader.style.width = "100vw";
setTimeout(() => {
loader.style.opacity = "0";
});
}
}
}, [tenantIsLoading, currentUserIsLoading]);
if (tenantIsLoading || currentUserIsLoading) return null;
function AppRouter() {
useFirstRouteLoad();
const isSafeCrash = useSelector(getSafeCrash);
const safeCrashCode: ERROR_CODES | undefined = useSelector(getSafeCrashCode);
return (
<Router history={history}>
<Suspense fallback={loadingIndicator}>
<RouteChangeListener />
{props.safeCrash && props.safeCrashCode ? (
{isSafeCrash && safeCrashCode ? (
<>
<ErrorPageHeader />
<ErrorPage code={props.safeCrashCode} />
<ErrorPage code={safeCrashCode} />
</>
) : (
<>
@ -200,17 +145,4 @@ function AppRouter(props: {
);
}
const mapStateToProps = (state: AppState) => ({
safeCrash: getSafeCrash(state),
safeCrashCode: getSafeCrashCode(state),
});
const mapDispatchToProps = (dispatch: any) => ({
getCurrentUser: () => dispatch(getCurrentUser()),
getFeatureFlags: () => dispatch(fetchFeatureFlagsInit()),
getCurrentTenant: () => dispatch(getCurrentTenant(false)),
initCurrentPage: () => dispatch(initCurrentPage()),
fetchProductAlert: () => dispatch(fetchProductAlertInit()),
});
export default connect(mapStateToProps, mapDispatchToProps)(AppRouter);
export default AppRouter;

View File

@ -151,7 +151,6 @@ const ActionTypes = {
RESET_SNIPING_MODE: "RESET_SNIPING_MODE",
RESET_EDITOR_REQUEST: "RESET_EDITOR_REQUEST",
RESET_EDITOR_SUCCESS: "RESET_EDITOR_SUCCESS",
INITIALIZE_CURRENT_PAGE: "INITIALIZE_CURRENT_PAGE",
INITIALIZE_EDITOR: "INITIALIZE_EDITOR",
INITIALIZE_EDITOR_SUCCESS: "INITIALIZE_EDITOR_SUCCESS",
REPORT_ERROR: "REPORT_ERROR",

View File

@ -27,7 +27,7 @@ export const isValidLicense = () => {
return true;
};
export const isTenantLoading = (state: AppState) => {
export const getIsTenantLoading = (state: AppState) => {
return state.tenant?.isLoading;
};

View File

@ -1,6 +1,9 @@
import React from "react";
import PageLoadingBar from "pages/common/PageLoadingBar";
import { retryPromise } from "utils/AppsmithUtils";
import { useSelector } from "react-redux";
import { getCurrentUserLoading } from "selectors/usersSelectors";
import { getIsTenantLoading } from "@appsmith/selectors/tenantSelectors";
const Page = React.lazy(() =>
retryPromise(
@ -12,6 +15,9 @@ const Page = React.lazy(() =>
);
const AdminSettingsLoader = (props: any) => {
const tenantIsLoading = useSelector(getIsTenantLoading);
const currentUserIsLoading = useSelector(getCurrentUserLoading);
if (tenantIsLoading || currentUserIsLoading) return null;
return (
<React.Suspense fallback={<PageLoadingBar />}>
<Page {...props} />

View File

@ -0,0 +1,48 @@
import { useEffect } from "react";
import useBrandingTheme from "../utils/hooks/useBrandingTheme";
import { useDispatch, useSelector } from "react-redux";
import { getCurrentUser } from "../actions/authActions";
import {
fetchFeatureFlagsInit,
fetchProductAlertInit,
} from "../actions/userActions";
import { getCurrentTenant } from "@appsmith/actions/tenantActions";
import { getIsTenantLoading } from "@appsmith/selectors/tenantSelectors";
import { getCurrentUserLoading } from "../selectors/usersSelectors";
export const useFirstRouteLoad = () => {
const dispatch = useDispatch();
const tenantIsLoading = useSelector(getIsTenantLoading);
const currentUserIsLoading = useSelector(getCurrentUserLoading);
useEffect(() => {
dispatch(getCurrentUser());
dispatch(fetchFeatureFlagsInit());
dispatch(getCurrentTenant());
dispatch(fetchProductAlertInit());
}, []);
useBrandingTheme();
// hide the top loader once the tenant is loaded
// Show app only after tenant is loaded (concerns theming and licence check)
useEffect(() => {
if (tenantIsLoading === false && currentUserIsLoading === false) {
const loader = document.getElementById("loader") as HTMLDivElement;
const appLoadBlock = document.getElementById(
"app-load-block",
) as HTMLDivElement;
if (loader) {
loader.style.width = "100vw";
setTimeout(() => {
loader.style.opacity = "0";
});
}
if (appLoadBlock) {
appLoadBlock.style.visibility = "visible";
}
}
}, [tenantIsLoading, currentUserIsLoading]);
};

View File

@ -17,7 +17,7 @@ import {
import { getSafeCrash } from "selectors/errorSelectors";
import { getCurrentUser } from "selectors/usersSelectors";
import { ANONYMOUS_USERNAME } from "constants/userConstants";
import { put, takeLatest, call, select } from "redux-saga/effects";
import { put, takeLatest, call, select, take } from "redux-saga/effects";
import {
ERROR_401,
ERROR_403,
@ -269,11 +269,15 @@ function logErrorSaga(action: ReduxAction<{ error: ErrorPayloadType }>) {
* this saga do some logic before actually setting safeCrash to true
*/
function* safeCrashSagaRequest(action: ReduxAction<{ code?: ERROR_CODES }>) {
const user: User | undefined = yield select(getCurrentUser);
let user: User | undefined = yield select(getCurrentUser);
if (!user) {
yield take(ReduxActionTypes.FETCH_USER_DETAILS_SUCCESS);
user = yield select(getCurrentUser);
}
const code = get(action, "payload.code");
// if user is not logged and the error is "PAGE_NOT_FOUND",
// redirecting user to login page with redirecTo param
// redirecting user to login page with redirectTo param
if (
get(user, "email") === ANONYMOUS_USERNAME &&
code === ERROR_CODES.PAGE_NOT_FOUND

View File

@ -23,11 +23,7 @@ import { resetCurrentApplication } from "@appsmith/actions/applicationActions";
import log from "loglevel";
import * as Sentry from "@sentry/react";
import { resetRecentEntities } from "actions/globalSearchActions";
import {
initAppViewer,
initEditor,
resetEditorSuccess,
} from "actions/initActions";
import { resetEditorSuccess } from "actions/initActions";
import {
getCurrentPageId,
getIsEditorInitialized,
@ -42,7 +38,7 @@ import type AppEngine from "entities/Engine";
import { AppEngineApiError } from "entities/Engine";
import AppEngineFactory from "entities/Engine/factory";
import type { ApplicationPagePayload } from "@appsmith/api/ApplicationApi";
import { getSearchQuery, updateSlugNamesInURL } from "utils/helpers";
import { updateSlugNamesInURL } from "utils/helpers";
import { generateAutoHeightLayoutTreeAction } from "actions/autoHeightActions";
import { safeCrashAppRequest } from "../actions/errorActions";
import { resetSnipingMode } from "actions/propertyPaneActions";
@ -50,16 +46,7 @@ import {
setExplorerActiveAction,
setExplorerPinnedAction,
} from "actions/explorerActions";
import {
isEditorPath,
isViewerPath,
} from "@appsmith/pages/Editor/Explorer/helpers";
import { APP_MODE } from "../entities/App";
import {
GIT_BRANCH_QUERY_KEY,
matchBuilderPath,
matchViewerPath,
} from "../constants/routes";
import type { APP_MODE } from "../entities/App";
import AnalyticsUtil from "utils/AnalyticsUtil";
import { getAppMode } from "@appsmith/selectors/applicationSelectors";
@ -220,42 +207,6 @@ function* appEngineSaga(action: ReduxAction<AppEnginePayload>) {
});
}
function* eagerPageInitSaga() {
const url = window.location.pathname;
const search = window.location.search;
if (isEditorPath(url)) {
const {
params: { applicationId, pageId },
} = matchBuilderPath(url);
const branch = getSearchQuery(search, GIT_BRANCH_QUERY_KEY);
if (pageId) {
yield put(
initEditor({
pageId,
applicationId,
branch,
mode: APP_MODE.EDIT,
}),
);
}
} else if (isViewerPath(url)) {
const {
params: { applicationId, pageId },
} = matchViewerPath(url);
const branch = getSearchQuery(search, GIT_BRANCH_QUERY_KEY);
if (applicationId || pageId) {
yield put(
initAppViewer({
applicationId,
branch,
pageId,
mode: APP_MODE.PUBLISHED,
}),
);
}
}
}
export default function* watchInitSagas() {
yield all([
takeLeading(
@ -267,6 +218,5 @@ export default function* watchInitSagas() {
),
takeLatest(ReduxActionTypes.RESET_EDITOR_REQUEST, resetEditorSaga),
takeEvery(URL_CHANGE_ACTIONS, updateURLSaga),
takeEvery(ReduxActionTypes.INITIALIZE_CURRENT_PAGE, eagerPageInitSaga),
]);
}