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

chore(v6): change to React automatic runtime #8926

Merged
merged 2 commits into from
Jul 19, 2023

Conversation

jtoar
Copy link
Contributor

@jtoar jtoar commented Jul 18, 2023

This change is a long time coming (~three years... https://legacy.reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html) and is a change that our v6 Vite config already makes.

The post I linked to above explains the change well and I recommend reading it. But here's an example of the difference in dist output:

image

Note that if a user has custom React preset config, this PR has no affect, making it less worrisome to add to a release.

@Tobbe I know we discussed earlier, but remind me what the motivation for this change was? That modern tools rely on it right?

Here's the Babel docs for completeness: https://babeljs.io/docs/babel-plugin-transform-react-jsx

@jtoar jtoar added the release:breaking This PR is a breaking change label Jul 18, 2023
@jtoar jtoar added this to the v6.0.0 milestone Jul 18, 2023
@jtoar jtoar requested a review from Tobbe July 18, 2023 01:20
@Tobbe
Copy link
Member

Tobbe commented Jul 18, 2023

My use case is I'm using https://theme-ui.com. When I switched to using Vite as the bundler in my project theme-ui stopped working.
I read their docs https://theme-ui.com/sx-prop and they now recommend using the @jsxImportSource pragma together with the automatic runtime setting.
Emotion is moving in this direction as well emotion-js/emotion#2662

Our Vite configuration is already using the new(ish) automatic runtime. Bringing Webpack up to this as well will make it easier for users who decide to stay on Webpack for now to change to Vite later when we decide to stop supporting Webpack.

Copy link
Member

@Tobbe Tobbe left a comment

Choose a reason for hiding this comment

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

This is the same change I did when I tried this on my own project.
LGTM!

name: "editJob"`)
expect(outputNoStaticImports).toContain(
normalizeStr(
`}), /*#__PURE__*/(0, _jsxRuntime.jsx)(_router.Route, {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The main difference in these tests is that they look for /*#__PURE__*/(0, _jsxRuntime.jsx) instead of .createElement

Comment on lines +12 to +18
function normalizeStr(str: string) {
return str
.split('\n')
.map((line) => line.trim())
.join('\n')
.trim()
}
Copy link
Contributor Author

@jtoar jtoar Jul 19, 2023

Choose a reason for hiding this comment

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

I was getting failures for what I could only assume was spacing, so after some trial and error where I'd add a space or a tab (which I entertained for too long), I just decided to normalize everything

@jtoar jtoar merged commit f6acf92 into main Jul 19, 2023
@jtoar jtoar deleted the ds-v6/switch-to-react-automatic-runtime branch July 19, 2023 22:11
jtoar added a commit that referenced this pull request Jul 19, 2023
This change is a long time coming (~three years...
https://legacy.reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html)
and is a change that our v6 Vite config already makes.

The post I linked to above explains the change well and **I recommend
reading it**. But here's an example of the difference in dist output:

<img width="2012" alt="image"
src="https://github.com/redwoodjs/redwood/assets/32992335/7f1a6e38-42c1-407f-8317-8a8ceb7ce672">

Note that if a user has custom React preset config, this PR has no
affect, making it less worrisome to add to a release.

@Tobbe I know we discussed earlier, but remind me what the motivation
for this change was? That modern tools rely on it right?

Here's the Babel docs for completeness:
https://babeljs.io/docs/babel-plugin-transform-react-jsx
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:breaking This PR is a breaking change
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants