fix(plugin-react): duplicate __self prop and __source prop
Description
fix: #9363
Both __source and __self are automatically set when using the automatic runtime.
So we should avoid transform a ReactElement containing __source or __self props to jsx.
Additional context
What is the purpose of this pull request?
- [x] Bug fix
- [ ] New Feature
- [ ] Documentation update
- [ ] Other
Before submitting the PR, please make sure you do the following
- [x] Read the Contributing Guidelines.
- [x] Read the Pull Request Guidelines and follow the Commit Convention.
- [x] Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
- [x] Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g.
fixes #123). - [x] Ideally, include relevant tests that fail without this PR but pass with it.
Would you explain why this was happening only when NODE_ENV=development?
Also IMO we should remove __source and __self after transformed to JSX instead of avoiding the transform.
Would you explain why this was happening only when
NODE_ENV=development?
https://github.com/babel/babel/blob/029fa1778296ad2e854313d2582aacf3a0d74d43/packages/babel-plugin-transform-react-jsx/src/create-plugin.ts#L564-L573
Because the __self and __source props are automatically added only when NODE_ENV=development.
Also IMO we should remove
__sourceand__selfafter transformed to JSX instead of avoiding the transform.
Not quite agree, we should not remove the user-defined __source or __self and use the auto-generated one, we should make it consistent
I did some research, I think the root of the problem is sentry, which is probably using an older bundler that causes the npm source code to contain __self and __source props, so we just need to get it to run successfully.
Because the
__selfand__sourceprops are automatically added only whenNODE_ENV=development.
I did some research, I think the root of the problem is
sentry, which is probably using an older bundler that causes the npm source code to contain__selfand__sourceprops, we just need to get it to run successfully.
@sentry/react's bundle includes __source/__self regardless of NODE_ENV and babel-plugin-transform-react-jsx injects them when NODE_ENV=development. So it's only duplicated when NODE_ENV=development. Is my understanding correct?
Not quite agree, we should not remove the user-defined
__sourceor__selfand use the auto-generated one, we should make it consistent
I think this is a build consistency / development info / runtime performance trade-off.
- avoid transform with both
NODE_ENV=developmentandNODE_ENV=prodcution:__source/__selfis correct but we cannot gain performance of automatic runtime - avoid transform with
NODE_ENV=developmentbut trasformNODE_ENV=prodcution:__source/__selfis correct but build betweenNODE_ENVis not consistent - transform with both
NODE_ENV=developmentandNODE_ENV=prodcution:__source/__selfis not correct
About the runtime perf, I'm not sure how much it affects. https://github.com/alloc/vite-react-jsx#faq
@sentry/react's bundle includes__source/__selfregardless ofNODE_ENVandbabel-plugin-transform-react-jsxinjects them whenNODE_ENV=development. So it's only duplicated whenNODE_ENV=development. Is my understanding correct?
Exactly!
I think this is a build consistency / development info / runtime performance trade-off.
- avoid transform with both
NODE_ENV=developmentandNODE_ENV=prodcution:__source/__selfis correct but we cannot gain performance of automatic runtime- avoid transform with
NODE_ENV=developmentbut trasformNODE_ENV=prodcution:__source/__selfis correct but build betweenNODE_ENVis not consistent- transform with both
NODE_ENV=developmentandNODE_ENV=prodcution:__source/__selfis not correct
I think there are very few cases where a bundle contains __self or __source, so we just need to ignore the cases that contain __self or __source. This shouldn't have much of an impact.
It is worth mentioning that before #9386, I found that most of the ReactElements were not converted to jsx.🤣
@aleclarson Thank you, Can you review this?
I am hoping this PR will be merged. I am having the same problem with a combination of Ladle and react-bootstrap.