-
Notifications
You must be signed in to change notification settings - Fork 11.6k
perf: timezone select component #26980
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.
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
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No issues found across 1 file
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/features/components/timezone-select/TimezoneSelect.tsx">
<violation number="1" location="packages/features/components/timezone-select/TimezoneSelect.tsx:157">
P2: Inline `styles` object is recreated on every render, which defeats memoization and can cause unnecessary re-renders in react-select. Keep the styles object memoized so its reference is stable across renders.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Devin AI is addressing Cubic AI's review feedbackA Devin session has been created to address the issues identified by Cubic AI. |
| timezones={{ | ||
| ...(props.data ? addTimezonesToDropdown(data) : {}), | ||
| ...(isWebTimezoneSelect ? addTimezonesToDropdown(additionalTimezones) : {}), | ||
| }} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This would have created new object on every render
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/features/components/timezone-select/TimezoneSelect.tsx">
<violation number="1" location="packages/features/components/timezone-select/TimezoneSelect.tsx:88">
P2: Memoizing `data` by `props.data?.length` can return stale timezone options when the array contents change but length doesn’t. Depend on the full `props.data` reference (or remove memoization) so updates propagate correctly.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
packages/features/components/timezone-select/TimezoneSelect.tsx
Outdated
Show resolved
Hide resolved
Devin AI is addressing Cubic AI's review feedbackA Devin session has been created to address the issues identified by Cubic AI. |
The useMemo dependency on props.data?.length could return stale timezone options when the array contents change but the length stays the same. Using the full props.data reference ensures updates propagate correctly. Fix identified by Cubic AI (confidence 9/10) Co-Authored-By: unknown <>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1 issue found across 1 file (changes from recent commits).
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="packages/features/components/timezone-select/TimezoneSelect.tsx">
<violation number="1" location="packages/features/components/timezone-select/TimezoneSelect.tsx:88">
P2: Memoizing `data` only on `props.data?.length` can leave stale timezone options when the array contents change but the length doesn’t. Depend on `props.data` itself (or a stable hash) so updates propagate correctly.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
packages/features/components/timezone-select/TimezoneSelect.tsx
Outdated
Show resolved
Hide resolved
Devin AI is addressing Cubic AI's review feedbackNew feedback has been sent to the existing Devin session. ✅ Pushed commit |
The useMemo dependency on props.data?.length could return stale timezone options when the array contents change but the length stays the same. Using the full props.data reference ensures updates propagate correctly. Fix identified by Cubic AI (confidence 9/10) Co-Authored-By: unknown <>
What does this PR do?
Saw this
vscomponent in Profile which I think is react-select's internal componentsMandatory Tasks (DO NOT REMOVE)
How should this be tested?
Summary by cubic
Optimized the timezone select to reduce re-renders and speed up search and typing. Also fixed stale options by correcting memoization dependencies.
Written for commit 7aac94d. Summary will update on new commits.