ref(core)!: Cleanup internal types, including `ReportDialogOptions` by mydea · Pull Request #14861 · getsentry/sentry-javascript

mydea · 2024-12-30T14:00:49Z

These are small changes, cleaning up outdated (I believe?) TODOs, and some internal type stuff.

mydea · 2024-12-30T14:01:13Z

packages/core/src/integrations/inboundfilters.ts

 }

 function _isDeniedUrl(event: Event, denyUrls?: Array<string | RegExp>): boolean {
-  // TODO: Use Glob instead?


@AbhiPrasad I think this is not really relevant anymore, is it? 🤔

Yeah we can remove!

mydea · 2024-12-30T14:01:40Z

packages/core/src/utils-hoist/time.ts

 /**
 * Returns a timestamp in seconds since the UNIX epoch using the Date API.
- *
- * TODO(v8): Return type should be rounded.


@AbhiPrasad I think this would be pretty breaking, I do not think we actually want to reduce fidelity here? Or am I missing something?

I think we added this because relay assumes that seconds comes in as integers, not floats. Let's ping ingest to double check.

but this would lose a whole lot of accuracy, wouldn't it? Or how would you then send sub-second timestamps?

mydea · 2024-12-30T14:01:59Z

packages/opentelemetry/src/utils/contextData.ts


 /**
 * Get the context related to a scope.
- * TODO v8: Use this for the `trace` functions.


We are already doing this, so all good :D

github-actions · 2024-12-30T14:07:46Z

size-limit report 📦

Path	Size	% Change	Change
@sentry/browser	22.74 KB	-	-
@sentry/browser - with treeshaking flags	21.51 KB	-	-
@sentry/browser (incl. Tracing)	35.39 KB	+0.01%	+1 B 🔺
@sentry/browser (incl. Tracing, Replay)	72.09 KB	-0.01%	-1 B 🔽
@sentry/browser (incl. Tracing, Replay) - with treeshaking flags	62.62 KB	-0.01%	-1 B 🔽
@sentry/browser (incl. Tracing, Replay with Canvas)	76.36 KB	-	-
@sentry/browser (incl. Tracing, Replay, Feedback)	88.38 KB	+0.01%	+1 B 🔺
@sentry/browser (incl. Feedback)	39.07 KB	-	-
@sentry/browser (incl. sendFeedback)	27.42 KB	-	-
@sentry/browser (incl. FeedbackAsync)	32.25 KB	-	-
@sentry/react	25.49 KB	-	-
@sentry/react (incl. Tracing)	38.15 KB	+0.01%	+1 B 🔺
@sentry/vue	27.08 KB	-	-
@sentry/vue (incl. Tracing)	37.24 KB	+0.01%	+2 B 🔺
@sentry/svelte	22.86 KB	-	-
CDN Bundle	24.12 KB	-	-
CDN Bundle (incl. Tracing)	35.69 KB	-0.02%	-4 B 🔽
CDN Bundle (incl. Tracing, Replay)	70.24 KB	-0.01%	-5 B 🔽
CDN Bundle (incl. Tracing, Replay, Feedback)	75.45 KB	-0.01%	-4 B 🔽
CDN Bundle - uncompressed	70.47 KB	-	-
CDN Bundle (incl. Tracing) - uncompressed	106.04 KB	-0.01%	-1 B 🔽
CDN Bundle (incl. Tracing, Replay) - uncompressed	216.92 KB	-0.01%	-1 B 🔽
CDN Bundle (incl. Tracing, Replay, Feedback) - uncompressed	229.67 KB	-0.01%	-1 B 🔽
@sentry/nextjs (client)	38.28 KB	+0.01%	+2 B 🔺
@sentry/sveltekit (client)	35.9 KB	+0.01%	+2 B 🔺
@sentry/node	161.47 KB	+0.01%	+1 B 🔺
@sentry/node - without tracing	97.28 KB	+0.01%	+1 B 🔺
@sentry/aws-serverless	127.12 KB	-	-

View base workflow run

codecov · 2024-12-30T14:08:03Z

❌ 5 Tests Failed:

Tests completed	Failed	Passed	Skipped
697	5	692	298

View the top 2 failed tests by shortest run time

client/app-dir-pageloads.test.ts should create a pageload transaction when the `app` directory is used

Stack Traces | 120s run time

app-dir-pageloads.test.ts:4:5 should create a pageload transaction when the `app` directory is used

client/click-error.test.ts should send error for faulty click handlers

Stack Traces | 120s run time

click-error.test.ts:4:5 should send error for faulty click handlers

View the full list of 1 ❄️ flaky tests

client-app-routing-instrumentation.test.ts Creates a navigation transaction for app router routes

Flake rate in main: 9.09% (Passed 30 times, Failed 3 times)

Stack Traces | 30s run time

client-app-routing-instrumentation.test.ts:19:5 Creates a navigation transaction for app router routes

To view more test analytics, go to the Test Analytics Dashboard
📢 Thoughts on this report? Let us know!

AbhiPrasad · 2024-12-30T15:26:19Z

packages/core/src/integrations/inboundfilters.ts

 }

 function _isDeniedUrl(event: Event, denyUrls?: Array<string | RegExp>): boolean {
-  // TODO: Use Glob instead?


Yeah we can remove!

AbhiPrasad · 2024-12-30T15:27:06Z

packages/core/src/utils-hoist/time.ts

 /**
 * Returns a timestamp in seconds since the UNIX epoch using the Date API.
- *
- * TODO(v8): Return type should be rounded.


I think we added this because relay assumes that seconds comes in as integers, not floats. Let's ping ingest to double check.

AbhiPrasad · 2024-12-30T15:27:28Z

packages/browser/src/sdk.ts

-  // TODO(v9): Change this to  [key: string]: unknkown;
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  [key: string]: any;
+  [key: string]: unknown;


I think this requires a changelog entry.

Added a changelog entry!

AbhiPrasad

ReportDialogOptions changes needs a changelog entry because we are breaking the types.

AbhiPrasad · 2025-01-02T15:12:05Z

packages/core/src/api.ts

@@ -47,9 +47,7 @@ export function getEnvelopeEndpointWithUrlEncodedAuth(dsn: DsnComponents, tunnel
 export function getReportDialogEndpoint(
  dsnLike: DsnLike,
  dialogOptions: {


I wonder if this should be typed with ReportDialogOptions

I moved the type to core so we can reuse this here!

mydea requested review from AbhiPrasad and andreiborza December 30, 2024 14:00

mydea self-assigned this Dec 30, 2024

mydea commented Dec 30, 2024

View reviewed changes

AbhiPrasad reviewed Dec 30, 2024

View reviewed changes

mydea changed the title ~~ref(core): Cleanup todos & internal types~~ ref(core): Cleanup internal types Dec 30, 2024

mydea force-pushed the fn/cleanup-todos branch from 9ffc282 to 2b01487 Compare December 30, 2024 15:32

AbhiPrasad approved these changes Jan 2, 2025

View reviewed changes

mydea force-pushed the fn/cleanup-todos branch from 2b01487 to a36df93 Compare January 3, 2025 12:02

mydea changed the title ~~ref(core): Cleanup internal types~~ ref(core)!: Cleanup internal types, including ReportDialogOptions Jan 3, 2025

mydea requested a review from AbhiPrasad January 3, 2025 12:43

mydea added 2 commits January 8, 2025 15:32

ref(core): Cleanup internal types

0624132

move report dialog options type to core & add to changelog

3bfcc1a

mydea force-pushed the fn/cleanup-todos branch from a36df93 to 3bfcc1a Compare January 8, 2025 14:32

mydea merged commit 2327642 into develop Jan 9, 2025

mydea deleted the fn/cleanup-todos branch January 9, 2025 08:29

Uh oh!

Conversation

mydea commented Dec 30, 2024

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!

github-actions bot commented Dec 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

size-limit report 📦

Uh oh!

codecov bot commented Dec 30, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 5 Tests Failed:

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!

AbhiPrasad 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

github-actions bot commented Dec 30, 2024 •

edited

Loading

codecov bot commented Dec 30, 2024 •

edited

Loading