-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
ref(ui): deprecate useProjects() without slugs #104462
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: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -21,7 +21,14 @@ export interface ProfilingBreadcrumbsProps { | |
| } | ||
|
|
||
| function ProfilingBreadcrumbs({organization, trails}: ProfilingBreadcrumbsProps) { | ||
| const {projects} = useProjects(); | ||
| // Extract project slugs from trails that have them | ||
| const projectSlugs = trails | ||
| .filter( | ||
| (trail): trail is ProfileSummaryTrail | FlamegraphTrail => | ||
| trail.type === 'profile summary' || trail.type === 'flamechart' | ||
| ) | ||
| .map(trail => trail.payload.projectSlug); | ||
| const {projects} = useProjects({slugs: projectSlugs}); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. theres gotta be a better way to do breadcrumbs than pulling in projects like this - might need to just look at the payload. if its got slug, it might have name? |
||
| const crumbs = useMemo( | ||
| () => trails.map(trail => trailToCrumb(trail, {organization, projects})), | ||
| [organization, trails, projects] | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -87,7 +87,7 @@ export default function TeamKeyTransactionFieldWrapper({ | |
| transactionName, | ||
| ...props | ||
| }: WrapperProps) { | ||
| const {projects} = useProjects(); | ||
| const {projects} = useProjects({slugs: projectSlug ? [projectSlug] : []}); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. similar to the profiling one, project should be empty if no projectId? thus no projects defined? |
||
| const project = projects.find(proj => proj.slug === projectSlug); | ||
|
|
||
| // All these fields need to be defined in order to toggle a team key | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -149,10 +149,25 @@ async function fetchProjects( | |
| * This hook also provides a way to select specific project slugs, and search | ||
| * (type-ahead) for more projects that may not be in the project store. | ||
| * | ||
| * NOTE: Currently ALL projects are always loaded, but this hook is designed | ||
| * for future-compat in a world where we do _not_ load all projects. | ||
| * DEPRECATED USAGE: Calling useProjects() without the `slugs` parameter is | ||
| * deprecated. This pattern assumes all projects are loaded in the store, | ||
| * which will not be true once we remove the all_projects=1 bootstrap fetch. | ||
| * | ||
| * RECOMMENDED: Always pass specific slugs you need: | ||
| * const {projects} = useProjects({slugs: [group.project.slug]}); | ||
| * | ||
| * This ensures projects are fetched on-demand if not already in the store. | ||
| */ | ||
| function useProjects({limit, slugs, orgId: propOrgId}: Options = {}) { | ||
| if (process.env.NODE_ENV !== 'production' && !slugs) { | ||
| // eslint-disable-next-line no-console | ||
| console.warn( | ||
| 'useProjects() called without slugs parameter. ' + | ||
| 'This usage is deprecated and may break when all_projects=1 is removed. ' + | ||
| 'Pass specific slugs to ensure projects are fetched on-demand.' | ||
| ); | ||
| } | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Deprecation warning fires repeatedly on every renderThe
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. seems fine |
||
|
|
||
| const api = useApi(); | ||
|
|
||
| const organization = useOrganization({allowNull: true}); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,12 +16,12 @@ interface Props { | |
| } | ||
|
|
||
| export function TransactionCell({project, transaction, transactionMethod}: Props) { | ||
| const projects = useProjects(); | ||
| const {projects} = useProjects({slugs: project ? [project] : []}); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. q: is project optional here becauase its not always present in the cells? does that mean we only need to look up project when its present similar to the other concerns with props above? |
||
| const organization = useOrganization(); | ||
| const location = useLocation(); | ||
| const {view} = useDomainViewFilters(); | ||
|
|
||
| const projectId = projects.projects.find(p => p.slug === project)?.id; | ||
| const projectId = projects.find(p => p.slug === project)?.id; | ||
|
|
||
| const searchQuery = new MutableSearch(''); | ||
| if (transactionMethod) { | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -234,7 +234,6 @@ function useFetchGroupDetails(): FetchGroupDetailsState { | |
| const navigate = useNavigate(); | ||
| const defaultIssueEvent = useDefaultIssueEvent(); | ||
| const hasStreamlinedUI = useHasStreamlinedUI(); | ||
| const {projects} = useProjects(); | ||
|
|
||
| const [allProjectChanged, setAllProjectChanged] = useState<boolean>(false); | ||
|
|
||
|
|
@@ -261,6 +260,9 @@ function useFetchGroupDetails(): FetchGroupDetailsState { | |
| refetch: refetchGroupCall, | ||
| } = useGroup({groupId}); | ||
|
|
||
| const groupProjectSlug = groupData?.project?.slug; | ||
| const {projects} = useProjects({slugs: groupProjectSlug ? [groupProjectSlug] : []}); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if project slug isnt found we should hard error on the group - it means the project's missing/deleted/similar |
||
|
|
||
| /** | ||
| * TODO(streamline-ui): Remove this whole hook once the legacy UI is removed. The streamlined UI exposes the | ||
| * filters on the page so the user is expected to clear it themselves, and the empty state is actually expected. | ||
dcramer marked this conversation as resolved.
Show resolved
Hide resolved
dcramer marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -157,7 +157,8 @@ export function SpanNodeDetails( | |
| const {node, organization} = props; | ||
| const location = useLocation(); | ||
| const theme = useTheme(); | ||
| const {projects} = useProjects(); | ||
| const projectSlug = node.value.project_slug ?? node.event?.projectSlug; | ||
| const {projects} = useProjects({slugs: projectSlug ? [projectSlug] : []}); | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hard error on missing slug i believe - someone correct me if im wrong |
||
| const issues = TraceTree.UniqueIssues(node); | ||
|
|
||
| const parentTransaction = isEAPSpanNode(node) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -61,11 +61,10 @@ export function UptimeNodeDetails( | |
| const {node} = props; | ||
| const location = useLocation(); | ||
| const theme = useTheme(); | ||
| const {projects} = useProjects(); | ||
| const projectSlug = node.value.project_slug ?? node.event?.projectSlug; | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hard error on missing slug i believe - someone correct me if im wrong |
||
| const {projects} = useProjects({slugs: projectSlug ? [projectSlug] : []}); | ||
|
|
||
| const project = projects.find( | ||
| proj => proj.slug === (node.value.project_slug ?? node.event?.projectSlug) | ||
| ); | ||
| const project = projects.find(proj => proj.slug === projectSlug); | ||
|
|
||
| return ( | ||
| <UptimeSpanNodeDetails | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -195,7 +195,7 @@ export function ContinuousProfileProvider({ | |
| setProfile, | ||
| }: ContinuousProfileProviderProps) { | ||
| const api = useApi(); | ||
| const {projects} = useProjects(); | ||
| const {projects} = useProjects({slugs: projectSlug ? [projectSlug] : []}); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Missing fetch state check causes false Sentry errorsThe |
||
|
|
||
| useLayoutEffect(() => { | ||
| if (!profileMeta) { | ||
|
|
||
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.
i think this one should just be empty if no projectId is set