chore: Remove deprecatedRouteProps for /pull/ #103223
Conversation
ryan953
changed the title
github-actions
bot
added
the
Scope: Frontend
|
@sentry review |
![sentry[bot]](https://avatars.githubusercontent.com/in/12637?s=60&v=4){kind=small}
| )} | ||
| > | ||
| <NoProjectMessage organization={organization}> | ||
| <UrlParamBatchProvider>{children}</UrlParamBatchProvider> | ||
| <UrlParamBatchProvider> | ||
| <Outlet /> | ||
| </UrlParamBatchProvider> | ||
| </NoProjectMessage> | ||
| </Feature> | ||
| ); | ||
| } |
There was a problem hiding this comment.
The migration from a children prop to React Router v6's <Outlet /> component is correctly implemented. The component now properly renders nested routes. However, ensure that the UrlParamBatchProvider is the appropriate context wrapper for the <Outlet />. If this provider was previously wrapping only specific child components, verify that wrapping the entire <Outlet /> is the intended behavior and won't cause unintended side effects on all nested routes.
Severity: MEDIUM
🤖 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/pullRequest/index.tsx#L1-L35
Potential issue: The migration from a children prop to React Router v6's `<Outlet />`
component is correctly implemented. The component now properly renders nested routes.
However, ensure that the `UrlParamBatchProvider` is the appropriate context wrapper for
the `<Outlet />`. If this provider was previously wrapping only specific child
components, verify that wrapping the entire `<Outlet />` is the intended behavior and
won't cause unintended side effects on all nested routes.
Did we get this right? 👍 / 👎 to inform future reviews.
| }; | ||
|
|
||
| function PullRequestContainer({children}: Props) { | ||
| export default function PullRequestContainer() { |
There was a problem hiding this comment.
The change from a named export to a default export is appropriate. However, verify that no other files are importing PullRequestContainer as a named import (e.g., import { PullRequestContainer }). Search the codebase for any named imports to ensure compatibility. Only the route configuration should reference this component via default export.
Severity: MEDIUM
🤖 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/pullRequest/index.tsx#L11
Potential issue: The change from a named export to a default export is appropriate.
However, verify that no other files are importing `PullRequestContainer` as a named
import (e.g., `import { PullRequestContainer }`). Search the codebase for any named
imports to ensure compatibility. Only the route configuration should reference this
component via default export.
Did we get this right? 👍 / 👎 to inform future reviews.
This route only uses
children, so it's quick to convert to<Outlet>