Skip to content
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

reactComponentAnnotation. I use an unintended value for a prop. #492

Open
jin-Pro opened this issue Feb 26, 2024 · 5 comments
Open

reactComponentAnnotation. I use an unintended value for a prop. #492

jin-Pro opened this issue Feb 26, 2024 · 5 comments

Comments

@jin-Pro
Copy link

jin-Pro commented Feb 26, 2024

Problem Statement

Hello, I am a software developer using @sentry/vite-plugin.

I am using reactComponentAnnotation in sentryVitePlugin.

I was thinking of injecting the corresponding value into html's attribute through babel.

However, I saw data-sentry-component and data-sentry-source-file in react component prop,
Because of this value I faced error.

I think this function interferes with the spread operation, which is a basic function of javascript.
Because of this, I argue that this feature should be changed.

---------------- Additional explanation ---------------------

I used sentry library in my project.

The packaged version of the library used in my project.

"vite": "^5.4.10",
"@sentry/react": "^8.35.0",
"@sentry/vite-plugin": "^2.22.6",
Sentry's reactComponentAnnotation feature injects properties like data-sentry-component and data-sentry-source-file into component props.

This can cause unintended errors when using methods like Object.keys() to process props, as unexpected keys are included.

For example, in logic where components are dynamically rendered based on specific keys, these additional properties can lead to unintended behavior.

const testObjValue = {
  a: ComponentA,
  b: ComponentB,
};

type ComponentType = {
  a: any;
  b: any;
};

const Component = (props: ComponentType) => {
  const keys = Object.keys(props) as ['a', 'b'];
  return (
    <ul>
      {keys.map((key) => {
        const ChildComponent = testObjValue[key];
        return <ChildComponent key={key} />; 
      })}
    </ul>
  );
};

Solution Brainstorm

The reactComponentAnnotation feature of Sentry should be modified to directly add attributes only to the HTML tags in JSX.
This approach avoids modifying the component's props and affects only the HTML DOM, effectively preventing unintended side effects.

@Lms24
Copy link
Member

Lms24 commented Feb 26, 2024

Hi @jin-Pro, thanks for writing in!

If you could provide a minimal reproducible example for the issue we'd greatly appreciate it! Also please feel free to propose a concrete solution.

Gonna tag @0Calories to take a look please, thx!

@AbhiPrasad AbhiPrasad transferred this issue from getsentry/sentry-javascript Feb 26, 2024
@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 2 Feb 26, 2024
@mattleong
Copy link

mattleong commented Aug 16, 2024

I think this function interferes with the spread operation, which is a basic function of javascript.

Can confirm that is an issue using the webpack plugin as well. Specifically, we were unable to spread Sets. 🤷‍♂

Resolution is to not use reactComponentAnnotation in the mean time.

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Aug 16, 2024
@chargome
Copy link
Member

@mattleong could you specify what you are trying to do exactly, or better, provide a small reproducible example?

@getsantry
Copy link

getsantry bot commented Nov 23, 2024

This issue has gone three weeks without activity. In another week, I will close it.

But! If you comment or otherwise update it, I will reset the clock, and if you remove the label Waiting for: Community, I will leave it alone ... forever!


"A weed is but an unloved flower." ― Ella Wheeler Wilcox 🥀

@jin-Pro
Copy link
Author

jin-Pro commented Nov 27, 2024

I used sentry library in my project.

The packaged version of the library used in my project.

  • "vite": "^5.4.10",
  • "@sentry/react": "^8.35.0",
  • "@sentry/vite-plugin": "^2.22.6",

Sentry's reactComponentAnnotation feature injects properties like data-sentry-component and data-sentry-source-file into component props.

This can cause unintended errors when using methods like Object.keys() to process props, as unexpected keys are included.

For example, in logic where components are dynamically rendered based on specific keys, these additional properties can lead to unintended behavior.

const testObjValue = {
  a: ComponentA,
  b: ComponentB,
};

type ComponentType = {
  a: any;
  b: any;
};

const Component = (props: ComponentType) => {
  const keys = Object.keys(props) as ['a', 'b'];
  return (
    <ul>
      {keys.map((key) => {
        const ChildComponent = testObjValue[key];
        return <ChildComponent key={key} />; 
      })}
    </ul>
  );
};

The reactComponentAnnotation feature of Sentry should be modified to directly add attributes only to the HTML tags in JSX.
This approach avoids modifying the component's props and affects only the HTML DOM, effectively preventing unintended side effects.

@Lms24 @0Calories

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: No status
Status: Waiting for: Product Owner
Development

No branches or pull requests

6 participants