ref(ui): Remove unused optional arguments and fields by JonasBa · Pull Request #122373 · getsentry/sentry

JonasBa · 2026-08-20T21:52:16Z

Follow-up to #122234. This diff comes from an experimental CLI. It looks at every place a component or function is used, checks which props and optional arguments are actually passed in, and flags the ones that never are.

Summary

Remove unused optional arguments and object fields across static/app, static/gsApp, and static/gsAdmin.
Keep public component/hook/class options (onError, onSuccess, onClose, size, mutation option bags, etc.) even when nothing passes them today.
Where an internal helper still dropped a defaulted argument, lift the old default to a file-level const so the name stays (TRUNCATE_TEXT_MAX_LENGTH, tick intervals, etc.).
Restore Duration HTML attribute forwarding: Tooltip skipWrapper clones hover handlers onto the child, so dropping {...props} broke the duration tooltip.

Test plan

Spot-check a few high-traffic call sites that lost optional args.
Confirm public hooks still accept onError/onSuccess (useHandleAssigneeChange, detector form submit, assign-issue mutation).
CI typecheck / frontend tests.

Legal Boilerplate

Look, I get it. The entity doing business as "Sentry" was incorporated in the State of Delaware in 2015 as Functional Software, Inc. and is gonna need some rights from me in order to utilize my contributions in this here PR. So here's the deal: I retain all rights, title and interest in and to my contributions, and by keeping this boilerplate intact I confirm that Sentry can use, modify, copy, and redistribute my contributions, under Sentry's choice of terms.

Made with Cursor

nsdeschenes

Had an async chat regarding concerns around losing semantics when dropping arguments that have defaults values.

@JonasBa is gonna take another stab at this, to help reduce that lost value

JonasBa · 2026-08-25T17:19:29Z

@nicholas-codecov @TkDodo I've reran the mod to preserve semantics, let me know if there's anything else I should change 🙏🏼

JonasBa · 2026-08-25T18:34:57Z

Follow-up production-only unused-arg layer: #122624

JonasBa · 2026-08-25T18:36:45Z

@TkDodo @nicholas-codecov I have a followup that treats test usage as dead code (similar to knip), which hopefully drops another 3k loc. #122624

cursor · 2026-08-26T06:31:40Z

-
-    config.onHighlight?.(datum);
-  }
+  onHighlight(_dataIndex: number): void {}


Time series highlight callback is a no-op

Medium Severity

Area and Line onHighlight implementations were replaced with empty bodies, so config.onHighlight never runs. Hover/highlight handlers on those plottables, including the documented time-series widget story, will not fire.

Additional Locations (1)

static/app/views/dashboards/widgets/timeSeriesWidget/plottables/line.tsx#L24-L25

^{Reviewed by Cursor Bugbot for commit dc8a6d8. Configure here.}

TkDodo

went over 1/3 of the code, will look at the rest later, but there are some issues I found with this:

JonasBa · 2026-08-26T17:52:02Z

@TkDodo addressed the open comments on this pass:

restored setIsMetricsExtractedData(mapKey, value: boolean) so dashboard widgets still record extracted-metrics
dropped unused traceInfo from SpanTreeModel / WaterfallModel
simplified the discover event-details query object
removed redundant enabled: true on useProfileEventsStats

Left shouldEscapeTagValue's options arg — escapeTagValueForSearch still passes {allowArrayValue, forceQuote}.

JonasBa

went over the rest of this PR (the files after the first pass). same class of issues: empty callbacks that still exist on the public type, and leftover undefined / enabled: true after dropping an unused arg.

JonasBa · 2026-08-26T18:02:17Z

-
-    config.onHighlight?.(datum);
-  }
+  onHighlight(_dataIndex: number): void {}


this is wrong. onHighlight is now a no-op, but ContinuousTimeSeriesConfig.onHighlight is still a public option and Bars.onHighlight still forwards it:

sentry/static/app/views/dashboards/widgets/timeSeriesWidget/plottables/bars.tsx

Lines 23 to 32 in aa48e02

onHighlight(dataIndex: number): void {

const {config = {}} = this;

const datum = this.timeSeries.values.at(dataIndex);

if (!datum) {

return;

}

config.onHighlight?.(datum);

}

the Highlighting story also says all plottables support this. restore the merged-index lookup (incomplete series prepend a point, so dataIndex is not a raw series index) or drop the option from the shared config and from Area/Line together.

JonasBa · 2026-08-26T18:02:22Z

-
-    config.onHighlight?.(datum);
-  }
+  onHighlight(_seriesDataIndex: number): void {}


same as Area — empty onHighlight while Bars still implements the shared config. restore or delete both together.

JonasBa · 2026-08-26T18:02:24Z

        projectSlug: project.slug,
      },
-      {enabled: hasFeatureFlag}
+      {enabled: true}


this can also go because enabled: true is the default. leftover from dropping hasFeatureFlag — current callers (cursorIntegrationCta / claudeCodeIntegrationCta) never passed featureFlag, so this is just noise.

JonasBa · 2026-08-26T18:02:26Z

          subject: ctxKey,
          value: data[ctxKey],
-          meta: meta?.[ctxKey]?.[''],
+          meta: undefined,


this is technically correct, but meta is never passed — getPlatformContextData only calls getSpringContextData({data}). same leftover as homepage: undefined:

Suggested change

meta: undefined,

value: data[ctxKey],

JonasBa · 2026-08-26T18:02:28Z

 ): TokenResult<Token.VALUE_TEXT_LIST> | null {
  try {
-    return parse(value, {TokenConverter, config, startRule: 'text_in_list'});
+    return parse(value, {TokenConverter, config: undefined, startRule: 'text_in_list'});


config: undefined is leftover from dropping the unused config arg. callers only pass the string (valueCombobox, replaceCommaSeparatedValue), and the grammar already defaults config = {}. omit the key.

JonasBa · 2026-08-26T18:02:29Z

-        // In group-id mode the page-filters project selection must not hide
-        // the deep-linked issue — the backend still enforces access.
-        project: pinnedGroupIds ? -1 : (projects ?? -1),
+        group: undefined,


group: undefined and enabled: true below are leftovers from dropping groupIds / enabled. the only caller is autofixIssuesDemo/index.tsx with {query, cursor}. don't put undefined into the query object; enabled: true is the react-query default.

JonasBa · 2026-08-26T18:02:31Z

-      useCache,
-      enabled,
+      useCache: true,
+      enabled: true,


same leftover as useProfileEventsStats: enabled was dropped from this hook because useIssueListFilterKeys never passed it, then inlined enabled: true. useFetchOrganizationTags already treats omitted enabled as on, and useCache already defaults to true. drop both always-true args here and on the issue-platform call.

…vided. Co-authored-by: Cursor <cursoragent@cursor.com>

Co-authored-by: Cursor <cursoragent@cursor.com>

…ooltips. Co-authored-by: Cursor <cursoragent@cursor.com>

Rescanned with unused-props after Dominik's merge. Delete optional arguments and fields nothing still provides, and inline the old defaults so runtime behavior stays the same.

Keep unused optional callbacks and component/hook options in place so call sites can still pass onError/onSuccess/onClose later. Where an internal helper inlined a default, lift it to a file-level const so the old argument name is not lost (e.g. truncateText maxLength).

The widget viewer modal still passes aliases into the loading placeholder. Removing the optional prop was a type error.

Restore the metrics extracted-data setter boolean, drop leftover traceInfo plumbing, and remove remaining homepage/enabled noise. Co-authored-by: Cursor <cursoragent@cursor.com>

Area/Line still advertise ContinuousTimeSeriesConfig.onHighlight; empty bodies lost that. Also remove leftover enabled:true / undefined keys. Co-authored-by: Cursor <cursoragent@cursor.com>

TkDodo · 2026-08-27T07:44:12Z

bugbot run

cursor

✅ Bugbot reviewed your changes and found no new issues!

1 issue from previous review remains unresolved.

Comment @cursor review or bugbot run to trigger another review on this PR

^{Reviewed by Cursor Bugbot for commit db4ffbd. Configure here.}

TkDodo

This is 🔥 . Just went over it again and have a few more findings:

TkDodo · 2026-08-27T07:49:37Z

        this.addOp('OR');
      }
-      this.addFilterValue(key, values[i]!, shouldEscape);
+      this.addFilterValue(key, values[i]!, true);


true is the default value in this.addFilterValue as well:

Suggested change

this.addFilterValue(key, values[i]!, true);

this.addFilterValue(key, values[i]!);

TkDodo · 2026-08-27T07:55:00Z

-    `${makeAutomationBasePathname(orgSlug)}new/?${qs.stringify(query)}`
-  );
+export const makeAutomationCreatePathname = (orgSlug: string) => {
+  return normalizeUrl(`${makeAutomationBasePathname(orgSlug)}new/?${qs.stringify({})}`);


qs.stringify({}) is a sus leftover. I guess we just want:

Suggested change

return normalizeUrl(`${makeAutomationBasePathname(orgSlug)}new/?${qs.stringify({})}`);

return normalizeUrl(`${makeAutomationBasePathname(orgSlug)}new/`);

TkDodo · 2026-08-27T07:59:02Z

@@ -123,7 +115,7 @@ function WidgetLayout(props: Widget) {
      )}

      {props.Footer && (
-        <FooterWrapper noPadding={props.noFooterPadding}>
+        <FooterWrapper>


Header and FooterWrapper still have the prop defined even though they never get it passed ,e.g.:

export const FooterWrapper = styled('div')<{noPadding?: boolean}>` margin: 0; border-top: 1px solid ${p => p.theme.tokens.border.primary}; padding: ${p => p.noPadding ? 0 : `${p.theme.space.md} ${p.theme.space.xl} ${p.theme.space.md} ${p.theme.space.xl}`}; `;

TkDodo · 2026-08-27T08:08:29Z

  }
  if (billingInterval === 'monthly') {
-    return toDollars(cents * 12, decimals);
+    return toDollars(cents * 12, 0);


decimals defaults to 0 so we don’t need to pass it#

Suggested change

return toDollars(cents * 12, 0);

return toDollars(cents * 12);

maybe we should have a no-exact-default-value lint rule that checks if you pass a hardcoded value to an optional param that is equal to its default value 🤔 ?

of course I nerd-sniped myself into making one:

feat(eslint): no-redundant default argument #122820

TkDodo · 2026-08-27T08:08:36Z

+    return toDollars(cents * 12, 0);
  }
-  return toDollars(cents, decimals);
+  return toDollars(cents, 0);


Suggested change

return toDollars(cents, 0);

return toDollars(cents);

TkDodo · 2026-08-27T08:10:58Z

 }: {
  product: AddOnCategory;
  withPunctuation: boolean;
-  includedBudget?: string;


just saying it’s wild that this isn’t flagged by some lint rule already. like the prop isn’t destructed at all so why is it there lol

TkDodo · 2026-08-27T08:11:55Z

      size="sm"
      href={href}
-      to={to ?? ''}
+      to=""


I think we can omit this param if we pass an href

TkDodo · 2026-08-27T08:13:30Z

+  let effectiveReservedSpend: number | undefined;

  if (
    (effectiveReservedCpe === undefined || effectiveReservedSpend === undefined) &&


at this point, effectiveReservedSpend is always undefined

github-actions Bot added the Scope: Frontend Automatically applied to PRs that change frontend components label Aug 20, 2026

nsdeschenes added the Trigger: getsentry tests Once code is reviewed: apply label to PR to trigger getsentry tests label Aug 21, 2026

nsdeschenes marked this pull request as ready for review August 21, 2026 13:25

nsdeschenes requested review from a team as code owners August 21, 2026 13:25

nsdeschenes self-assigned this Aug 21, 2026

nsdeschenes marked this pull request as draft August 21, 2026 13:51

github-actions Bot removed the Trigger: getsentry tests Once code is reviewed: apply label to PR to trigger getsentry tests label Aug 25, 2026

nsdeschenes requested changes Aug 25, 2026

View reviewed changes

This was referenced Aug 25, 2026

ref(ui): Drop optional args unused outside tests JonasBa/sentry#1

Closed

ref(ui): Drop optional args unused outside tests #122624

Draft

JonasBa marked this pull request as ready for review August 25, 2026 18:22

JonasBa requested a review from a team as a code owner August 25, 2026 18:22

JonasBa marked this pull request as draft August 25, 2026 18:23

TkDodo approved these changes Aug 26, 2026

View reviewed changes

TkDodo marked this pull request as ready for review August 26, 2026 06:26

cursor Bot reviewed Aug 26, 2026

View reviewed changes

TkDodo requested changes Aug 26, 2026

View reviewed changes

JonasBa commented Aug 26, 2026

View reviewed changes

Jonas Badalic and others added 9 commits August 26, 2026 11:27

ref(ui): Remove unused optional arguments and fields that nothing pro…

894f5c7

…vided. Co-authored-by: Cursor <cursoragent@cursor.com>

ref(ui): Unexport symbols left unused after optional-arg cleanup.

b4654fd

Co-authored-by: Cursor <cursoragent@cursor.com>

ref(ui): Apply eslint and oxfmt after unused optional-arg deletions.

a98366b

Co-authored-by: Cursor <cursoragent@cursor.com>

fix(ui): Restore Duration HTML attribute forwarding for skipWrapper t…

e4c4458

…ooltips. Co-authored-by: Cursor <cursoragent@cursor.com>

ref(ui): Drop unused optional args that landed after the master merge.

34fa280

Rescanned with unused-props after Dominik's merge. Delete optional arguments and fields nothing still provides, and inline the old defaults so runtime behavior stays the same.

fix(ui): Restore TableWidgetVisualization.LoadingPlaceholder aliases.

260a059

The widget viewer modal still passes aliases into the loading placeholder. Removing the optional prop was a type error.

ref(ui): Address review comments on unused-arg cleanup.

0966339

Restore the metrics extracted-data setter boolean, drop leftover traceInfo plumbing, and remove remaining homepage/enabled noise. Co-authored-by: Cursor <cursoragent@cursor.com>

ref(ui): Restore plottable onHighlight and drop leftover unused args.

db4ffbd

Area/Line still advertise ContinuousTimeSeriesConfig.onHighlight; empty bodies lost that. Also remove leftover enabled:true / undefined keys. Co-authored-by: Cursor <cursoragent@cursor.com>

JonasBa force-pushed the jonas/unused-optional-args-cleanup branch from bebe8a5 to db4ffbd Compare August 26, 2026 18:28

cursor Bot reviewed Aug 27, 2026

View reviewed changes

TkDodo reviewed Aug 27, 2026

View reviewed changes

	onHighlight(dataIndex: number): void {
	const {config = {}} = this;
	const datum = this.timeSeries.values.at(dataIndex);

	if (!datum) {
	return;
	}

	config.onHighlight?.(datum);
	}

	this.addFilterValue(key, values[i]!, true);
	this.addFilterValue(key, values[i]!);

	return normalizeUrl(`${makeAutomationBasePathname(orgSlug)}new/?${qs.stringify({})}`);
	return normalizeUrl(`${makeAutomationBasePathname(orgSlug)}new/`);

	return toDollars(cents * 12, 0);
	return toDollars(cents * 12);

Uh oh!

Uh oh!

Conversation

JonasBa commented Aug 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Legal Boilerplate

Uh oh!

nsdeschenes left a comment

Choose a reason for hiding this comment

Uh oh!

JonasBa commented Aug 25, 2026

Uh oh!

JonasBa commented Aug 25, 2026

Uh oh!

JonasBa commented Aug 25, 2026

Uh oh!

Uh oh!

cursor Bot Aug 26, 2026

Choose a reason for hiding this comment

Time series highlight callback is a no-op

Uh oh!

TkDodo left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JonasBa commented Aug 26, 2026

Uh oh!

JonasBa left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TkDodo commented Aug 27, 2026

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

TkDodo left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

JonasBa commented Aug 20, 2026 •

edited

Loading