10000 chore(core): Mark `Span.getSpanJSON` method as internal by Lms24 · Pull Request #10197 · getsentry/sentry-javascript · GitHub
[go: up one dir, main page]
Skip to content

chore(core): Mark Span.getSpanJSON method as internal #10197

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jan 16, 2024
Merged

chore(core): Mark Span.getSpanJSON method as internal #10197

merged 1 commit into from
Jan 16, 2024

Conversation

Lms24
Copy link
Member
@Lms24 Lms24 commented Jan 16, 2024 •
edited
Loading

This small PR just clarifies the API status of the Span.getSpanJSON method. This method is purely purposed for internal usage and users should not rely on it but instead on spanToJSON.

Why should it stay on the Span class anyway?

  • The alternative would be to access private properties of Span class instances externally, which isn't clean or particularly safe either.
  • Given above, we'd need to exclude all private property names from being mangled by Terser when we build our CDN bundles. Which also opens us up to potential errors in the future where we'd add private fields but forget to add the mangling exclusion.

I originally intended to deprecate this method but after discussing this, we decided to just clarify the internal purpose instead. The way we use this function right now is pretty safe, compared to the alternative mentioned above.

ref #10184

@Lms24 Lms24 changed the title chore(core): Update Span.getSpanJSON description as internal chore(core): Mark Span.getSpanJSON method for internal usage Jan 16, 2024
@Lms24 Lms24 changed the title chore(core): Mark Span.getSpanJSON method for internal usage chore(core): Mark Span.getSpanJSON method as internal Jan 16, 2024
@Lms24 Lms24 requested a review from mydea January 16, 2024 12:34
@Lms24 Lms24 mentioned this pull request Jan 16, 2024
Closed
@Lms24 Lms24 self-assigned this Jan 16, 2024
@Lms24 Lms24 enabled auto-merge (squash) January 16, 2024 12:47
@Lms24 Lms24 merged commit 6077cb9 into develop Jan 16, 2024
@Lms24 Lms24 deleted the lms/chore-core-clarify-getSpanJSON-purpose branch January 16, 2024 12:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mydea mydea mydea approved these changes

Assignees

@Lms24 Lms24

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@Lms24 @mydea
0