Skip to content

fix(react-native): Enhance fragment detection for indirect references #767

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

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

antonis
Copy link

@antonis antonis commented Jul 18, 2025

@kahest kahest requested review from Lms24, AbhiPrasad and chargome July 21, 2025 08:22
Copy link
Member

@AbhiPrasad AbhiPrasad left a comment

Choose a reason for hiding this comment

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

overall approach seem good to me. Anecdotally traversing imports usually is not that expensive, but it might be nice to do a sanity check benchmark.

Program: {
enter(path, state) {
const fragmentContext = collectFragmentContext(path);
state['sentryFragmentContext'] = fragmentContext;
Copy link
Member

Choose a reason for hiding this comment

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

We should update AnnotationPluginPass to strongly type sentryFragmentContext.

@@ -151,7 +166,8 @@ function functionBodyPushAttributes(
componentName: string,
sourceFileName: string | undefined,
attributeNames: string[],
ignoredComponents: string[]
ignoredComponents: string[],
fragmentContext?: FragmentContext
Copy link
Member

Choose a reason for hiding this comment

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

Might be time to convert this to take an object instead of plain args (and also add a jsdoc to document)

Copy link
Member

@Lms24 Lms24 left a comment

Choose a reason for hiding this comment

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

Sounds reasonable to me -- thanks for adding all the tests!

Besides Abhi's feedback, could you take a look at the integration test? Perhaps we can add a fragment or two there as well (but happy to leave this up to you. If it's too much of a hassle that's fine as well)

@antonis
Copy link
Author

antonis commented Jul 21, 2025

Thank you both for the reviews and feedback. I really appreciate this 🙇
I'll iterate on it and mark the PR as ready for review 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Invalid prop data-sentry-element supplied to React.Fragment
3 participants