Skip to content

Conversation

@timfish
Copy link
Collaborator

@timfish timfish commented Jan 21, 2026

Rather than inject attributes into React components which can cause incompatibilities with props, this PR changes the plugin to instead inject attributes into every HTML element in the component root.

Downsides

Potentially larger bundle sizes because the attributes will get injected into more elements.

@github-actions
Copy link

github-actions bot commented Jan 21, 2026

Semver Impact of This PR

🟢 Patch (bug fixes)

📋 Changelog Preview

This is how your changes will appear in the changelog.
Entries from this PR are highlighted with a left border (blockquote style).


Bug Fixes 🐛

  • Inject component annotations into HTML elements rather than React components by timfish in #848

🤖 This preview updates automatically when you update the PR.

@timfish timfish marked this pull request as ready for review January 21, 2026 20:52
@timfish timfish force-pushed the timfish/fix/inject-annotations-into-html-elements branch 2 times, most recently from 729d328 to d498cc1 Compare January 21, 2026 20:59
@timfish timfish force-pushed the timfish/fix/inject-annotations-into-html-elements branch 2 times, most recently from 32c96e7 to 148b620 Compare January 21, 2026 21:45
@timfish timfish changed the title fix: Inject component annotations into HTML elements fix: Inject component annotations into HTML elements rather than React components Jan 21, 2026
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.

Thanks! I'll let other folks also review, especially also get perspective from ReactNative so that we're on the same page.

Copy link
Member

Choose a reason for hiding this comment

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

super-l: Should we refactor the tests to use toMatchInlineSnapshot? Feels way easier to me to associate which test corresponds to which snapshot. Also fine to tackle as a follow-up or not at all if I'm missing a reason for keeping it as-is. WDYT?

@Lms24 Lms24 requested review from antonis, chargome and s1gr1d January 22, 2026 11:42
Copy link
Member

@s1gr1d s1gr1d left a comment

Choose a reason for hiding this comment

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

LGTM but as discussed offline, we can put this behind a flag to reduce bundle size impact

@timfish timfish force-pushed the timfish/fix/inject-annotations-into-html-elements branch from a03a02a to 615a4ef Compare January 22, 2026 13:34
"@react-navigation",
];

export const REACT_NATIVE_ELEMENTS: string[] = [
Copy link
Contributor

@antonis antonis Jan 22, 2026

Choose a reason for hiding this comment

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

I think there are missing elements here (e.g. Pressable, TouchableOpacity that we are using in our samples). I'm not sure we can include everything in this list tbh. The users may also add custom components in their apps.
I was wondering if we can make this configurable so that we can add more elements on the RN SDK side or even allow for end users to extend.

Copy link
Contributor

@antonis antonis left a comment

Choose a reason for hiding this comment

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

As long as we do not pass experimentalInjectIntoHtml: true from the React Native SDK my understanding is that there is no change in behavior and the changes are safe to ship.
We can experiment with turning the feature ON and see the impact on our side. I think the main issue is the missing components before making this behavior default on the RN SDK.

@timfish timfish marked this pull request as draft January 22, 2026 15:56
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.

reactComponentAnnotation. I use an unintended value for a prop.

4 participants