Skip to content

Commit

Permalink
chore(v6): change to React automatic runtime (#8926)
Browse files Browse the repository at this point in the history
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
  • Loading branch information
jtoar committed Jul 19, 2023
1 parent 9f28039 commit 1007f89
Show file tree
Hide file tree
Showing 3 changed files with 132 additions and 72 deletions.
2 changes: 1 addition & 1 deletion babel.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ module.exports = {
].filter(Boolean),
},
],
'@babel/preset-react',
['@babel/preset-react', { runtime: 'automatic' }],
/**
* TODO(pc): w/ '@babel/plugin-transform-typescript' in plugins now, is '@babel/typescript' preset still needed?
*
Expand Down
200 changes: 130 additions & 70 deletions packages/internal/src/__tests__/nestedPages.test.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,26 @@
import path from 'path'

import { expect } from '@jest/globals'

import { getPaths } from '@redwoodjs/project-config'

import { prebuildWebFile } from '../build/babel/web'
import { cleanWebBuild } from '../build/web'

const FIXTURE_PATH = path.join(__dirname, 'fixtures/nestedPages')

function normalizeStr(str: string) {
return str
.split('\n')
.map((line) => line.trim())
.join('\n')
.trim()
}

describe('User specified imports, with static imports', () => {
let outputWithStaticImports: string | null | undefined
let outputNoStaticImports: string | null | undefined

beforeEach(() => {
process.env.RWJS_CWD = FIXTURE_PATH
cleanWebBuild()
Expand All @@ -21,16 +32,18 @@ describe('User specified imports, with static imports', () => {

beforeAll(() => {
process.env.RWJS_CWD = FIXTURE_PATH

const routesFile = getPaths().web.routes

outputWithStaticImports = prebuildWebFile(routesFile, {
prerender: true,
forJest: true,
})?.code
outputWithStaticImports &&= normalizeStr(outputWithStaticImports)

outputNoStaticImports = prebuildWebFile(routesFile, {
forJest: true,
})?.code
outputNoStaticImports &&= normalizeStr(outputNoStaticImports)
})

it('Imports layouts correctly', () => {
Expand All @@ -54,39 +67,47 @@ describe('User specified imports, with static imports', () => {
describe('static prerender imports', () => {
it('Adds loaders for non-nested pages', () => {
expect(outputWithStaticImports).toContain(
`const LoginPage = {
name: "LoginPage",
prerenderLoader: name => require("./pages/LoginPage/LoginPage"),
LazyComponent: (0, _react.lazy)(() => import( /* webpackChunkName: "LoginPage" */"./pages/LoginPage/LoginPage"))
}`
normalizeStr(
`const LoginPage = {
name: "LoginPage",
prerenderLoader: name => require("./pages/LoginPage/LoginPage"),
LazyComponent: (0, _react.lazy)(() => import( /* webpackChunkName: "LoginPage" */"./pages/LoginPage/LoginPage"))
}`
)
)

expect(outputWithStaticImports).toContain(
`const HomePage = {
name: "HomePage",
prerenderLoader: name => require("./pages/HomePage/HomePage"),
LazyComponent: (0, _react.lazy)(() => import( /* webpackChunkName: "HomePage" */"./pages/HomePage/HomePage"))
};`
normalizeStr(
`const HomePage = {
name: "HomePage",
prerenderLoader: name => require("./pages/HomePage/HomePage"),
LazyComponent: (0, _react.lazy)(() => import( /* webpackChunkName: "HomePage" */"./pages/HomePage/HomePage"))
};`
)
)
})
})

describe('dynamic build imports', () => {
it('Adds loaders for non-nested pages with __webpack_require__ and require.resolveWeak in prerenderLoader to not bundle the pages', () => {
expect(outputNoStaticImports).toContain(
`const LoginPage = {
name: "LoginPage",
prerenderLoader: name => __webpack_require__(require.resolveWeak("./pages/LoginPage/LoginPage")),
LazyComponent: (0, _react.lazy)(() => import( /* webpackChunkName: "LoginPage" */"./pages/LoginPage/LoginPage"))
}`
normalizeStr(
`const LoginPage = {
name: "LoginPage",
prerenderLoader: name => __webpack_require__(require.resolveWeak("./pages/LoginPage/LoginPage")),
LazyComponent: (0, _react.lazy)(() => import( /* webpackChunkName: "LoginPage" */"./pages/LoginPage/LoginPage"))
}`
)
)

expect(outputNoStaticImports).toContain(
`const HomePage = {
name: "HomePage",
prerenderLoader: name => __webpack_require__(require.resolveWeak("./pages/HomePage/HomePage")),
LazyComponent: (0, _react.lazy)(() => import( /* webpackChunkName: "HomePage" */"./pages/HomePage/HomePage"))
}`
normalizeStr(
`const HomePage = {
name: "HomePage",
prerenderLoader: name => __webpack_require__(require.resolveWeak("./pages/HomePage/HomePage")),
LazyComponent: (0, _react.lazy)(() => import( /* webpackChunkName: "HomePage" */"./pages/HomePage/HomePage"))
}`
)
)
})
})
Expand All @@ -97,22 +118,26 @@ describe('User specified imports, with static imports', () => {
it('Uses the user specified name for nested page', () => {
// Import statement: import NewJobPage from 'src/pages/Jobs/NewJobPage'
expect(outputWithStaticImports).toContain(
`const NewJobPage = {
name: "NewJobPage",
prerenderLoader: name => require("./pages/Jobs/NewJobPage/NewJobPage"),
LazyComponent: (0, _react.lazy)(() => import( /* webpackChunkName: "NewJobPage" */"./pages/Jobs/NewJobPage/NewJobPage"))
}`
normalizeStr(
`const NewJobPage = {
name: "NewJobPage",
prerenderLoader: name => require("./pages/Jobs/NewJobPage/NewJobPage"),
LazyComponent: (0, _react.lazy)(() => import( /* webpackChunkName: "NewJobPage" */"./pages/Jobs/NewJobPage/NewJobPage"))
}`
)
)
})

it('Uses the user specified custom default export import name for nested page', () => {
// Import statement: import BazingaJobProfilePageWithFunnyName from 'src/pages/Jobs/JobProfilePage'
expect(outputWithStaticImports).toContain(
`const BazingaJobProfilePageWithFunnyName = {
name: "BazingaJobProfilePageWithFunnyName",
prerenderLoader: name => require("./pages/Jobs/JobProfilePage/JobProfilePage"),
LazyComponent: (0, _react.lazy)(() => import( /* webpackChunkName: "BazingaJobProfilePageWithFunnyName" */"./pages/Jobs/JobProfilePage/JobProfilePage"))
}`
normalizeStr(
`const BazingaJobProfilePageWithFunnyName = {
name: "BazingaJobProfilePageWithFunnyName",
prerenderLoader: name => require("./pages/Jobs/JobProfilePage/JobProfilePage"),
LazyComponent: (0, _react.lazy)(() => import( /* webpackChunkName: "BazingaJobProfilePageWithFunnyName" */"./pages/Jobs/JobProfilePage/JobProfilePage"))
}`
)
)
})

Expand All @@ -128,12 +153,14 @@ describe('User specified imports, with static imports', () => {

it('Keeps using the user specified name when generating React component', () => {
// Generate react component still uses the user specified name
expect(outputWithStaticImports)
.toContain(`.createElement(_router.Route, {
path: "/job-profiles/{id:Int}",
page: BazingaJobProfilePageWithFunnyName,
name: "jobProfile"
})`)
expect(outputWithStaticImports).toContain(
normalizeStr(
`}), /*#__PURE__*/(0, _jsxRuntime.jsx)(_router.Route, {
path: "/job-profiles/{id:Int}",
page: BazingaJobProfilePageWithFunnyName,
name: "jobProfile"`
)
)
})
})

Expand All @@ -142,38 +169,63 @@ describe('User specified imports, with static imports', () => {
// Explicit import uses the specified import
// Has statement: import BazingaJobProfilePageWithFunnyName from 'src/pages/Jobs/JobProfilePage'
// The name of the import is not important without static imports
expect(outputNoStaticImports).toContain(`.createElement(_router.Route, {
path: "/job-profiles/{id:Int}",
page: _JobProfilePage["default"],
name: "jobProfile"
})`)
expect(outputNoStaticImports).toContain(
normalizeStr(
`}), /*#__PURE__*/(0, _jsxRuntime.jsx)(_router.Route, {
path: "/job-profiles/{id:Int}",
page: _JobProfilePage["default"],
name: "jobProfile"
})`
)
)
})

it("Uses the LazyComponent for a page that isn't imported", () => {
expect(outputNoStaticImports).toContain(`const HomePage = {
name: "HomePage",
prerenderLoader: name => __webpack_require__(require.resolveWeak("./pages/HomePage/HomePage")),
LazyComponent: (0, _react.lazy)(() => import( /* webpackChunkName: "HomePage" */"./pages/HomePage/HomePage"))
}`)
expect(outputNoStaticImports).toContain(`.createElement(_router.Route, {
path: "/",
page: HomePage,
name: "home"
})`)
expect(outputNoStaticImports).toContain(
normalizeStr(
`const HomePage = {
name: "HomePage",
prerenderLoader: name => __webpack_require__(require.resolveWeak("./pages/HomePage/HomePage")),
LazyComponent: (0, _react.lazy)(() => import( /* webpackChunkName: "HomePage" */"./pages/HomePage/HomePage"))
}`
)
)

expect(outputNoStaticImports).toContain(
normalizeStr(
`}), /*#__PURE__*/(0, _jsxRuntime.jsx)(_router.Route, {
path: "/",
page: HomePage,
name: "home"
})`
)
)
})

it('Should NOT add a LazyComponent for pages that have been explicitly loaded', () => {
expect(outputNoStaticImports).not.toContain(`const JobsJobPage = {
name: "JobsJobPage"`)
expect(outputNoStaticImports).not.toContain(
normalizeStr(
`const JobsJobPage = {
name: "JobsJobPage"`
)
)

expect(outputNoStaticImports).not.toContain(`const JobsNewJobPage = {
name: "JobsNewJobPage"`)
expect(outputNoStaticImports).not.toContain(
normalizeStr(
`const JobsNewJobPage = {
name: "JobsNewJobPage"`
)
)

expect(outputNoStaticImports).toContain(`.createElement(_router.Route, {
path: "/jobs",
page: _JobsPage["default"],
name: "jobs"
})`)
expect(outputNoStaticImports).toContain(
normalizeStr(
`}), /*#__PURE__*/(0, _jsxRuntime.jsx)(_router.Route, {
path: "/jobs",
page: _JobsPage["default"],
name: "jobs"
})`
)
)
})
})
})
Expand All @@ -183,20 +235,28 @@ describe('User specified imports, with static imports', () => {

expect(outputWithStaticImports).toContain('var _EditJobPage = require("')

expect(outputWithStaticImports).toContain(`const EditJobPage = {
name: "EditJobPage",
prerenderLoader: name => require("./pages/Jobs/EditJobPage/EditJobPage"),
LazyComponent: (0, _react.lazy)(() => import( /* webpackChunkName: "EditJobPage" */"./pages/Jobs/EditJobPage/EditJobPage"))
}`)
expect(outputWithStaticImports).toContain(
normalizeStr(
`const EditJobPage = {
name: "EditJobPage",
prerenderLoader: name => require("./pages/Jobs/EditJobPage/EditJobPage"),
LazyComponent: (0, _react.lazy)(() => import( /* webpackChunkName: "EditJobPage" */"./pages/Jobs/EditJobPage/EditJobPage"))
}`
)
)

expect(outputNoStaticImports).toContain(
'var _EditJobPage = _interopRequireWildcard('
)

expect(outputNoStaticImports).toContain(`.createElement(_router.Route, {
path: "/jobs/{id:Int}/edit",
page: _EditJobPage["default"],
name: "editJob"`)
expect(outputNoStaticImports).toContain(
normalizeStr(
`}), /*#__PURE__*/(0, _jsxRuntime.jsx)(_router.Route, {
path: "/jobs/{id:Int}/edit",
page: _EditJobPage["default"],
name: "editJob"`
)
)

// Should not generate a loader, because page was explicitly imported
expect(outputNoStaticImports).not.toMatch(
Expand Down
2 changes: 1 addition & 1 deletion packages/internal/src/build/babel/web.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,7 +132,7 @@ export const getWebSideBabelPresets = (options: Flags) => {
return []
}

let reactPresetConfig = undefined
let reactPresetConfig: babel.PluginItem = { runtime: 'automatic' }

// This is a special case, where @babel/preset-react needs config
// And using extends doesn't work
Expand Down

0 comments on commit 1007f89

Please sign in to comment.