refactor: [M3-7334] - Improve, clean up, and simplify app entrypoint and render tree logic #9844

bnussman-akamai · 2023-10-26T18:15:09Z

Description 📝

The goal of this PR is to simplify the entry point logic of Cloud Manager. Currently, the flow of index.tsx ➡️ AuthenticationWrapper.tsx ➡️ App.tsx ➡️ MainContent.tsx ➡️ ... is very complex and hard to follow, this ticket should help a little bit. There is a lot to clean up and improvement potential, but this PR is a solid start

Changes 🔄

Splashscreen simplification
AuthenticationWrapper is now a hook
IdentifyUser is now a hook
Uses less redux to track app loading state
Don't use unnecessarily complex preference toggle for Side Menu state, use preferences hooks directly

Preview 📷

Note

No UI changes are expected

How to test 🧪

Verification steps

Verify Cloud Manager loads like normal
Log out and verify you can log in without any issues
Verify that your remain logged in when you refresh
Verify Lish still works

As an Author I have considered 🤔

bnussman-akamai · 2023-10-26T19:10:32Z

packages/manager/src/store/authentication/authentication.reducer.ts

-export const defaultState: State = {
-  expiration: null,
-  loggedInAsCustomer: false,
-  scopes: null,
-  token: null,
-};


No reason to default to null when we can synchronously know the default values form local storage

bnussman-akamai · 2023-10-26T19:11:34Z

packages/manager/src/hooks/useInitialRequests.ts

This is basically AuthenticationWrapper converted into a hook

bnussman-akamai · 2023-10-26T19:13:19Z

packages/manager/src/components/AuthenticationWrapper/AuthenticationWrapper.test.tsx

This enzyme test isn't providing much value anymore. Many of the test cases are no longer accurate and we have plenty of e2e coverage that checks that the app loads properly

abailly-akamai

Awesome cleanup 🎉 . Adds a lot of clarity to our entry point. I think it's pretty close, I have not noticed any issue while testing locally but left some comments to see about improving a few things

packages/manager/src/MainContent.tsx

packages/manager/src/components/SplashScreen.tsx

packages/manager/src/hooks/useInitialRequests.ts

packages/manager/src/index.tsx

carrillo-erik · 2023-11-01T07:14:42Z

packages/manager/src/MainContent.tsx

+      desktop_sidebar_open: !preferences?.desktop_sidebar_open,
+    });
+  };
+


I really like this much cleaner approach.

carrillo-erik · 2023-11-01T07:25:49Z

packages/manager/src/store/authentication/authentication.reducer.ts

 const {
  expire: expiryInLocalStorage,
  scopes: scopesInLocalStorage,
  token: tokenInLocalStorage,
 } = authentication;

+const defaultToken = tokenInLocalStorage.get();


Is this const declared here to avoid typing out tokenInLocalStorage.get() in line 25 and 27?

packages/manager/src/MainContent.tsx

abailly-akamai

Thanks for addressing the feedback.

This looks ready as far as I could test it. ✅

Have not found issues locally and we probably want to have this sit in develop sooner than later so we can catch any potential issue before the next release.

tyler-akamai · 2023-11-01T13:59:00Z

Verify Cloud Manager loads like normal
Log out and verify you can log in without any issues
Verify that your remain logged in when you refresh
Verify Lish still works

tyler-akamai · 2023-11-01T15:06:22Z

packages/manager/src/MainContent.tsx

-                  className={cx(classes.content, {
-                    [classes.fullWidthContent]:
-                      desktopMenuIsOpen ||
-                      (desktopMenuIsOpen && desktopMenuIsOpen === true),


hmm this line looks a little odd, good job cleaning it up

tyler-akamai · 2023-11-01T15:09:44Z

packages/manager/src/features/TopMenu/TopMenu.tsx

-  } = props;
+  const { desktopMenuToggle, isSideMenuOpen, openSideMenu, username } = props;
+
+  const { loggedInAsCustomer } = useAuthentication();


Good cleanup

tyler-akamai · 2023-11-01T15:10:44Z

packages/manager/src/MainContent.tsx

+      desktop_sidebar_open: !preferences?.desktop_sidebar_open,
+    });
+  };
+


packages/manager/src/MainContent.tsx

tyler-akamai · 2023-11-01T15:31:37Z

packages/manager/src/hooks/useInitialRequests.ts

tyler-akamai · 2023-11-01T15:52:56Z

packages/manager/src/index.tsx

 import 'src/exceptionReporting';
 import Logout from 'src/layouts/Logout';
 import { setupInterceptors } from 'src/request';
 import { storeFactory } from 'src/store';

 import { App } from './App';
 import { LinodeThemeWrapper } from './LinodeThemeWrapper';
+import NullComponent from './components/NullComponent';
 import { loadDevTools, shouldEnableDevTools } from './dev-tools/load';


This is a small thing:
Should we dynamically import dev tools to ensure they arent part of the production bundle?

bnussman added 7 commits October 26, 2023 12:16

initial working

c4547cd

more fixes

2ee6d93

fixes to auth flow

c23ed1d

why default to null?

e36d0d3

clean up splash screen styles

6f078d5

clean up

daeec82

simplify things more

f5c02bb

bnussman-akamai self-assigned this Oct 26, 2023

bnussman-akamai requested a review from a team as a code owner October 26, 2023 18:15

bnussman-akamai requested review from jdamore-linode and tyler-akamai and removed request for a team October 26, 2023 18:15

bnussman-akamai added the Work in Progress label Oct 26, 2023

bnussman added 2 commits October 26, 2023 15:04

rename files

70e7edd

Added changeset: Clean up app entrypoint render logic

8e4dbac

bnussman-akamai removed the Work in Progress label Oct 26, 2023

bnussman-akamai commented Oct 26, 2023

View reviewed changes

bnussman-akamai requested a review from abailly-akamai October 26, 2023 19:57

improve useEffect logic

da46a32

abailly-akamai reviewed Oct 31, 2023

View reviewed changes

clean up / feeback

e7aa3c8

carrillo-erik reviewed Nov 1, 2023

View reviewed changes

packages/manager/src/MainContent.tsx Outdated Show resolved Hide resolved

abailly-akamai approved these changes Nov 1, 2023

View reviewed changes

tyler-akamai reviewed Nov 1, 2023

View reviewed changes

tyler-akamai approved these changes Nov 1, 2023

View reviewed changes

carrillo-erik approved these changes Nov 1, 2023

View reviewed changes

feedback and clean up

cce246f

bnussman-akamai merged commit 36e2adf into linode:develop Nov 1, 2023
13 checks passed

This was referenced Nov 8, 2023

Release v1.107.0 - release → staging #9889

Merged

Release v1.107.0 - staging → master #9898

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: [M3-7334] - Improve, clean up, and simplify app entrypoint and render tree logic #9844

refactor: [M3-7334] - Improve, clean up, and simplify app entrypoint and render tree logic #9844

bnussman-akamai commented Oct 26, 2023 •

edited

Loading

bnussman-akamai Oct 26, 2023

bnussman-akamai Oct 26, 2023

tyler-akamai Nov 1, 2023

bnussman-akamai Oct 26, 2023 •

edited

Loading

abailly-akamai left a comment

carrillo-erik Nov 1, 2023

tyler-akamai Nov 1, 2023

carrillo-erik Nov 1, 2023

bnussman-akamai Nov 1, 2023

abailly-akamai left a comment

tyler-akamai commented Nov 1, 2023 •

edited

Loading

tyler-akamai Nov 1, 2023

tyler-akamai Nov 1, 2023

tyler-akamai Nov 1, 2023

tyler-akamai Nov 1, 2023

tyler-akamai Nov 1, 2023

refactor: [M3-7334] - Improve, clean up, and simplify app entrypoint and render tree logic #9844

refactor: [M3-7334] - Improve, clean up, and simplify app entrypoint and render tree logic #9844

Conversation

bnussman-akamai commented Oct 26, 2023 • edited Loading

Description 📝

Changes 🔄

Preview 📷

How to test 🧪

Verification steps

As an Author I have considered 🤔

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bnussman-akamai Oct 26, 2023 • edited Loading

Choose a reason for hiding this comment

abailly-akamai left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

abailly-akamai left a comment

Choose a reason for hiding this comment

tyler-akamai commented Nov 1, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bnussman-akamai commented Oct 26, 2023 •

edited

Loading

bnussman-akamai Oct 26, 2023 •

edited

Loading

tyler-akamai commented Nov 1, 2023 •

edited

Loading