refactor: [M3-8981] - User Preferences optimization with selector pattern (Part 1) #11386

bnussman-akamai · 2024-12-09T16:42:45Z

Description 📝

Implements the "selector pattern" for our usePreferences hook
- This allows us to "subscribe" to certain parts of the preferences object, which help us prevent unnecessary re-renders when an unrelated preference is updated 🏎️
- See this post for a description of this React Query pattern

Note

This is just part 1, which introduces the pattern and uses it in a small number of places. I will follow up this PR with a part 2 (and maybe 3) to implement the pattern throughout the app.

Preview 📷

Important

No UI changes expected

How to test 🧪

Checkout this PR (or use a preview link)
Check the code
- Verify the new pattern is intuitive
- Check for code quality
To confirm this does not break anything
- Verify the primary navigation collapsed state is correctly saved in user preferences
- Verify the primary navigation collapsed state persists on related

Author Checklists

As an Author, to speed up the review process, I considered 🤔

👀 Doing a self review
❔ Our contribution guidelines
🤏 Splitting feature into small PRs
➕ Adding a changeset
🧪 Providing/improving test coverage
🔐 Removing all sensitive information from the code and PR description
🚩 Using a feature flag to protect the release
👣 Providing comprehensive reproduction steps
📑 Providing or updating our documentation
🕛 Scheduling a pair reviewing session
📱 Providing mobile support
♿ Providing accessibility support

I have read and considered all applicable items listed above.

As an Author, before moving this PR from Draft to Open, I confirmed ✅

All unit tests are passing
TypeScript compilation succeeded without errors
Code passes all linting rules

bnussman-akamai · 2024-12-09T18:21:46Z

packages/manager/src/features/Linodes/LinodesLanding/LinodesLanding.tsx

          preferenceKey="linodes_group_by_tag"
          preferenceOptions={[false, true]}
          toggleCallbackFn={sendGroupByAnalytic}
        >
          {({
            preference: linodesAreGrouped,
            togglePreference: toggleGroupLinodes,
-          }: PreferenceToggleProps<boolean>) => {


I improved the type-safety of PreferenceToggle. These explicit type annotations are no longer needed because the type is now properly inferred

bnussman-akamai · 2024-12-09T18:24:04Z

packages/manager/src/types/ManagerPreferences.ts

@@ -14,9 +14,11 @@ export interface DismissedNotification {
  label?: string;
 }

-export interface ManagerPreferences extends UserPreferences {


This extends was causing type-safety issues. It basically made ManagerPreferences extend Record<string, any>, which allowed any string key. Removing the extends allow us to be more strict.

bnussman-akamai · 2024-12-09T18:24:30Z

packages/manager/src/types/ManagerPreferences.ts

  avatarColor?: string;
  backups_cta_dismissed?: boolean;
+  collapsedSideNavProductFamilies?: number[];


These were types missing... Now that type-safety is better, developers will see type errors that will catch this

bnussman-akamai · 2024-12-09T18:26:55Z

packages/manager/src/queries/profile/preferences.ts

  });

 export const useMutatePreferences = (replace = false) => {
-  const { data: preferences } = usePreferences(!replace);


I removed this use of usePreferences and implemented mutationFn differently.

Using usePreferences here would cause any components that uses useMutatePreferences to re-render when preferences update. This change will eliminate that path to extra re-renders.

bnussman-akamai · 2024-12-09T18:28:25Z

packages/manager/src/components/PrimaryNav/PrimaryNav.tsx

-  const [collapsedAccordions, setCollapsedAccordions] = React.useState<
-    number[]
-  >(preferences?.collapsedSideNavProductFamilies ?? []);


We don't need to use extra state here. Because our user preferences optimistically update, we can essentially treat it as state.

bnussman-akamai · 2024-12-09T18:29:00Z

packages/manager/src/components/PreferenceToggle/PreferenceToggle.tsx

Had to make some changes here to improve type-safety. See other comment in LinodesLanding.tsx

bnussman-akamai · 2024-12-09T18:30:21Z

packages/manager/src/components/PrimaryNav/PrimaryNav.tsx

+  const { data: collapsedSideNavPreference } = usePreferences(
+    (preferences) => preferences?.collapsedSideNavProductFamilies
+  );


This is the new pattern in-use! ✨

Before: The primary nav would re-render when any preference was updated
After: The primary nav will only re-render when preferences.collapsedSideNavProductFamilies updates 🎉

This could be unrelated, but I’m noticing that PrimaryNav re-renders when navigating to Create Image, while it doesn’t re-render when selecting any products from the Side Nav or any other Create {Product}.

understood - it's a Create Image, but is PrimaryNav supposed to get re-rendered in such cases??

Yeah, that is unrelated. From my understanding, the primary nav will re-render when we change routes no matter what.

github-actions · 2024-12-10T22:06:05Z

Coverage Report: ✅
Base Coverage: 86.78%
Current Coverage: 86.78%

packages/manager/src/queries/profile/preferences.ts

Co-authored-by: Purvesh Makode <[email protected]>

mjac0bs

Nice, this was a cool improvement to see demoed and it will be good to optimize preference-use throughout the app!

Confirmed:

The new pattern is easy to understand
No regressions with primary navigation and collapsed state is correctly saved in user preferences
Toggling other preferences does not rerender the primary navigation; only changes to the collapsed state

Screen.Recording.2024-12-11.at.9.11.52.AM.mov

packages/manager/src/queries/profile/preferences.ts

Co-authored-by: Mariah Jacobs <[email protected]>

linode-gh-bot · 2024-12-12T02:05:49Z

Cloud Manager UI test results

🎉 469 passing tests on test run #9 ↗︎

❌ Failing	✅ Passing	↪️ Skipped	🕐 Duration
`0 Failing`	`469 Passing`	`2 Skipped`	`108m 25s`

cypress · 2024-12-12T14:24:32Z

Cloud Manager E2E Run #6959

Run Properties: Passed #6959 • abf05ca641: refactor: [M3-8981] - User Preferences optimization with selector pattern (Part ...

Project	`Cloud Manager E2E`
Branch Review	`develop`
Run status	`Passed #6959`
Run duration	`31m 11s`
Commit	`abf05ca641: refactor: [M3-8981] - User Preferences optimization with selector pattern (Part ...`
Committer	`Banks Nussman`
View all properties for this run ↗︎

Test results
Failures	`0`
Flaky	`3`
Pending	`2`
Skipped	`0`
Passing	`469`
View all changes introduced in this branch ↗︎

initial refactor

ad02ddb

bnussman-akamai added the React Query Relating to the transition to use React Query label Dec 9, 2024

bnussman-akamai self-assigned this Dec 9, 2024

fix type-safety issues

a31c62c

bnussman-akamai commented Dec 9, 2024

View reviewed changes

bnussman-akamai marked this pull request as ready for review December 9, 2024 18:31

bnussman-akamai requested a review from a team as a code owner December 9, 2024 18:31

bnussman-akamai requested review from mjac0bs and pmakode-akamai and removed request for a team December 9, 2024 18:31

bnussman added 2 commits December 10, 2024 10:46

Merge branch 'develop' into M3-8981-preference-optimizations

08308d9

Merge branch 'develop' into M3-8981-preference-optimizations

4fe21d3

pmakode-akamai reviewed Dec 11, 2024

View reviewed changes

packages/manager/src/queries/profile/preferences.ts Outdated Show resolved Hide resolved

bnussman-akamai and others added 2 commits December 11, 2024 09:44

Update packages/manager/src/queries/profile/preferences.ts

605fa4f

Co-authored-by: Purvesh Makode <[email protected]>

Merge branch 'develop' into M3-8981-preference-optimizations

b7e1bc9

bnussman-akamai requested a review from pmakode-akamai December 11, 2024 16:53

fix re-render at maincontent level @mjac0bs

d185cc4

mjac0bs approved these changes Dec 11, 2024

View reviewed changes

packages/manager/src/queries/profile/preferences.ts Show resolved Hide resolved

mjac0bs added the Add'tl Approval Needed Waiting on another approval! label Dec 11, 2024

Update packages/manager/src/queries/profile/preferences.ts

f4f9070

Co-authored-by: Mariah Jacobs <[email protected]>

pmakode-akamai approved these changes Dec 12, 2024

View reviewed changes

pmakode-akamai added Approved Multiple approvals and ready to merge! and removed Add'tl Approval Needed Waiting on another approval! labels Dec 12, 2024

bnussman-akamai merged commit abf05ca into linode:develop Dec 12, 2024
23 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

refactor: [M3-8981] - User Preferences optimization with selector pattern (Part 1) #11386

refactor: [M3-8981] - User Preferences optimization with selector pattern (Part 1) #11386

bnussman-akamai commented Dec 9, 2024

bnussman-akamai Dec 9, 2024 •

edited

Loading

bnussman-akamai Dec 9, 2024

bnussman-akamai Dec 9, 2024 •

edited

Loading

bnussman-akamai Dec 9, 2024

bnussman-akamai Dec 9, 2024

bnussman-akamai Dec 9, 2024

bnussman-akamai Dec 9, 2024

pmakode-akamai Dec 11, 2024 •

edited

Loading

bnussman-akamai Dec 11, 2024

github-actions bot commented Dec 10, 2024 •

edited

Loading

mjac0bs left a comment

linode-gh-bot commented Dec 12, 2024

cypress bot commented Dec 12, 2024

refactor: [M3-8981] - User Preferences optimization with selector pattern (Part 1) #11386

refactor: [M3-8981] - User Preferences optimization with selector pattern (Part 1) #11386

Conversation

bnussman-akamai commented Dec 9, 2024

Description 📝

Preview 📷

How to test 🧪

As an Author, to speed up the review process, I considered 🤔

As an Author, before moving this PR from Draft to Open, I confirmed ✅

bnussman-akamai Dec 9, 2024 • edited Loading

Choose a reason for hiding this comment

bnussman-akamai Dec 9, 2024

Choose a reason for hiding this comment

bnussman-akamai Dec 9, 2024 • edited Loading

Choose a reason for hiding this comment

bnussman-akamai Dec 9, 2024

Choose a reason for hiding this comment

bnussman-akamai Dec 9, 2024

Choose a reason for hiding this comment

bnussman-akamai Dec 9, 2024

Choose a reason for hiding this comment

bnussman-akamai Dec 9, 2024

Choose a reason for hiding this comment

pmakode-akamai Dec 11, 2024 • edited Loading

Choose a reason for hiding this comment

bnussman-akamai Dec 11, 2024

Choose a reason for hiding this comment

github-actions bot commented Dec 10, 2024 • edited Loading

mjac0bs left a comment

Choose a reason for hiding this comment

linode-gh-bot commented Dec 12, 2024

Cloud Manager UI test results

🎉 469 passing tests on test run #9 ↗︎

cypress bot commented Dec 12, 2024

Cloud Manager E2E Run #6959

bnussman-akamai Dec 9, 2024 •

edited

Loading

bnussman-akamai Dec 9, 2024 •

edited

Loading

pmakode-akamai Dec 11, 2024 •

edited

Loading

github-actions bot commented Dec 10, 2024 •

edited

Loading