Skip to content
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

ref(issue-list): Create copy of overview.tsx as a functional component #82784

Merged
merged 13 commits into from
Jan 6, 2025

Conversation

malwilley
Copy link
Member

@malwilley malwilley commented Dec 31, 2024

Adds a copy of overview.tsx (and tests) that is a functional component. I mostly copied it 1 for 1, so the resulting component is not much better than the original. We need a starting place first to start making more improvements.

This new component is only rendered if the issue-stream-functional-refactor flag is enabled. We will test this internally before rolling out more widely and deleting the original to reduce risk.

The most significant changes were around data fetching and caching, since converting things to useEffects resulted in some bugs and test failures that the original lifecycle methods managed to avoid somehow. For example, we are now using the requestParams object to determine if we should refetch instead of checking against previous props for everything individually.

I have also removed issue-stream-performance checks in this version of the component since that complicated things significantly.

Copy link

codecov bot commented Jan 2, 2025

Bundle Report

Changes will increase total bundle size by 18.7kB (0.06%) ⬆️. This is within the configured threshold ✅

Detailed changes
Bundle name Size Change
app-webpack-bundle-array-push 32.2MB 18.7kB (0.06%) ⬆️

@malwilley malwilley marked this pull request as ready for review January 2, 2025 20:54
@malwilley malwilley requested a review from a team as a code owner January 2, 2025 20:54
Copy link
Member

@scttcper scttcper left a comment

Choose a reason for hiding this comment

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

nice, lets start here and keep going

import withOrganization from 'sentry/utils/withOrganization';
import withPageFilters from 'sentry/utils/withPageFilters';
import withSavedSearches from 'sentry/utils/withSavedSearches';
import CustomViewsIssueListHeader from 'sentry/views/issueList/customViewsHeader';
Copy link
Member

@MichaelSun48 MichaelSun48 Jan 3, 2025

Choose a reason for hiding this comment

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

This is failing because I merged this PR that changed the component name - just replace this with

import IssueViewsIssueListHeader from 'sentry/views/issueList/issueViewsHeader';

and update the component below

@malwilley malwilley merged commit 7c678ff into master Jan 6, 2025
41 checks passed
@malwilley malwilley deleted the malwilley/ref/issue-list-overview-fc branch January 6, 2025 17:20
@malwilley malwilley added the Trigger: Revert Add to a merged PR to revert it (skips CI) label Jan 6, 2025
@getsentry-bot
Copy link
Contributor

PR reverted: fd477cd

getsentry-bot added a commit that referenced this pull request Jan 6, 2025
malwilley added a commit that referenced this pull request Jan 6, 2025
#82935)

Same as #82784 which got
reverted, with the addition of a type failure fix.

Adds a copy of overview.tsx (and tests) that is a functional component. I mostly copied it 1 for 1, so the resulting component is not much better than the original. We need a starting place first to start making more improvements.

This new component is only rendered if the issue-stream-functional-refactor flag is enabled. We will test this internally before rolling out more widely and deleting the original to reduce risk.

The most significant changes were around data fetching and caching, since converting things to useEffects resulted in some bugs and test failures that the original lifecycle methods managed to avoid somehow. For example, we are now using the requestParams object to determine if we should refetch instead of checking against previous props for everything individually.

I have also removed issue-stream-performance checks in this version of the component since that complicated things significantly.
@github-actions github-actions bot locked and limited conversation to collaborators Jan 22, 2025
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 Trigger: Revert Add to a merged PR to revert it (skips CI)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants