Skip to content

ref(settings): replace browserHistory with useNavigate in organizationProjects#110019

Merged
evanpurkhiser merged 1 commit into
masterfrom
evanpurkhiser/ref-settings-replace-browserhistory-with-usenavigate-in-organizationprojects
Mar 6, 2026
Merged

ref(settings): replace browserHistory with useNavigate in organizationProjects#110019
evanpurkhiser merged 1 commit into
masterfrom
evanpurkhiser/ref-settings-replace-browserhistory-with-usenavigate-in-organizationprojects

Conversation

@evanpurkhiser
Copy link
Copy Markdown
Member

No description provided.

@evanpurkhiser evanpurkhiser requested a review from a team March 5, 2026 23:31
@github-actions github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Mar 5, 2026
DEFAULT_DEBOUNCE_DURATION
),
[location.pathname, location.query]
[location.pathname, location.query, navigate]
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Bug: The useMemo for debouncedSearch has unstable dependencies (location.query and navigate), causing it to be recreated on every search, which resets the debounce timer.
Severity: MEDIUM

Suggested Fix

To stabilize the dependencies, remove location.query and navigate from the useMemo dependency array. The navigate function from useNavigate is guaranteed to be stable and does not need to be a dependency. The debounced function's logic does not depend on the current location.query.

Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.

Location: static/app/views/settings/organizationProjects/index.tsx#L102

Potential issue: The `debouncedSearch` function is created within a `useMemo` hook that
depends on `location.query`. However, `location.query` is regenerated as a new object on
every render because it is the result of `qs.parse()`. When a search triggers a
navigation, the component re-renders, `location.query` gets a new object reference, and
the `useMemo` hook re-creates the `debouncedSearch` function. This resets the debounce
timer, breaking the debouncing logic for any keystrokes following the first search. The
newly added `navigate` dependency is also unstable and contributes to this issue.

Did we get this right? 👍 / 👎 to inform future reviews.

Copy link
Copy Markdown
Contributor

@cursor cursor Bot left a comment

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes and found 1 potential issue.

Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

query: {...location.query, query: searchQuery, cursor: undefined},
},
{replace: true}
),
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Missing preventScrollReset option in navigate call

Low Severity

The browserHistory.replace shim automatically includes preventScrollReset: true (as documented in browserHistory.tsx with an explicit comment: "Note that useNavigate replace does not automatically prevent scroll reset"). Switching to navigate directly with only {replace: true} drops this behavior, which may cause unwanted scroll-to-top when typing in the search bar, since ScrollRestoration is active in the app layout.

Fix in Cursor Fix in Web

@evanpurkhiser evanpurkhiser merged commit bd38822 into master Mar 6, 2026
62 checks passed
@evanpurkhiser evanpurkhiser deleted the evanpurkhiser/ref-settings-replace-browserhistory-with-usenavigate-in-organizationprojects branch March 6, 2026 16:42
@github-actions github-actions Bot locked and limited conversation to collaborators Mar 22, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

Scope: Frontend Automatically applied to PRs that change frontend components

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants