fix(frontend): do not use redirects outside the react router #6739
Conversation
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
There was a problem hiding this comment.
👍 Looks good to me! Reviewed everything up to fa8f49e in 20 seconds
More details
- Looked at
50lines of code in2files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. frontend/src/AppRoutes/index.tsx:247
- Draft comment:
Consider refactoring the error handling and loading logic to avoid duplication and improve maintainability. - Reason this comment was not posted:
Confidence changes required:50%
The PR aims to replace the use of Redirect with history.push or history.replace. In the App component, the use of history.replace is consistent with the intent of the PR, which is to avoid using Redirect outside of React Router. However, the logic for handling errors and loading states is duplicated in the App component. This could be refactored to improve maintainability.
2. frontend/src/AppRoutes/index.tsx:256
- Draft comment:
Avoid using inline styles for the Spinner component. Consider using external stylesheets, CSS classes, or styled components instead. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_S83AJ2NCOLsPyeaC
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
There was a problem hiding this comment.
👍 Looks good to me! Incremental review on 2d5dd26 in 24 seconds
More details
- Looked at
26lines of code in2files - Skipped
0files when reviewing. - Skipped posting
2drafted comments based on config settings.
1. frontend/src/AppRoutes/pageComponents.ts:183
- Draft comment:
The webpack chunk name has been changed from 'SomethingWentWrong' to 'ErrorBoundaryFallback'. Ensure that this change is intentional and consistent across the codebase to avoid confusion. - Reason this comment was not posted:
Comment did not seem useful.
2. frontend/src/container/TopNav/DateTimeSelectionV2/config.ts:217
- Draft comment:
Avoid using thecomponent/index.tsxfile structure approach, as it makes it difficult to debug and find components using global search tools. - Reason this comment was not posted:
Comment was on unchanged code.
Workflow ID: wflow_lM6bNvcVhQSzE5BO
You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.
|
Build Error! No Linked Issue found. Please link an issue or mention it in the body using #<issue_id> |
Summary
history.replaceto something went wrong instead of using redirects outside react routerRelated Issues / PR's
contributes to - https://github.com/SigNoz/engineering-pod/issues/2123
Screenshots
NA
Affected Areas and Manually Tested Areas