feat: add automatic transform for jsx runtime #5973
Conversation
|
👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks! |
github-actions
bot
added
the
integration-tests: recommended
There was a problem hiding this comment.
Pull Request Overview
This PR switches the project’s Babel configuration to use React’s new automatic jsx runtime and updates component tests and implementations to accommodate the change. Key changes include updating the Babel and Rollup configurations, modifying component key handling in tests and render functions, and updating the changeset.
Reviewed Changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/tests/Pagination/Pagination.test.tsx | Updated renderPage to explicitly pass a key prop to the link component. |
| packages/react/src/FilteredActionList/FilteredActionListWithModernActionList.tsx | Refactored key extraction in the mapping of action list items. |
| packages/react/src/Autocomplete/AutocompleteMenu.tsx | Adjusted key handling in ActionList.Item to work with TS definitions. |
| packages/react/rollup.config.mjs | Updated Babel preset configuration to use the automatic jsx runtime. |
| packages/react/babel.config.js | Updated Babel preset configuration to use the automatic jsx runtime. |
| .changeset/wise-gifts-see.md | Documented the jsx transform update in the changeset. |
| // @ts-expect-error this is defined in the items above but is | ||
| // missing in TS |
There was a problem hiding this comment.
Consider updating the TypeScript type definitions for the items to include the 'key' property instead of suppressing the error with @ts-expect-error.
| // @ts-expect-error this is defined in the items above but is | |
| // missing in TS |
Copilot uses AI. Check for mistakes.
| @@ -31,7 +31,7 @@ describe('Pagination', () => { | |||
| <Pagination | |||
| pageCount={10} | |||
| currentPage={1} | |||
| renderPage={({number, ...props}) => <ReactRouterLikeLink to={`#${number}`} {...props} />} | |||
| renderPage={({key, number, ...props}) => <ReactRouterLikeLink key={key} to={`#${number}`} {...props} />} | |||
There was a problem hiding this comment.
[nitpick] Ensure that the 'key' prop provided here is unique and aligns with React's recommendations for maintaining element identity during re-renders.
| renderPage={({key, number, ...props}) => <ReactRouterLikeLink key={key} to={`#${number}`} {...props} />} | |
| renderPage={({key, number, ...props}) => <ReactRouterLikeLink key={number} to={`#${number}`} {...props} />} |
Copilot uses AI. Check for mistakes.
| const key = itemKey ?? item.id?.toString() ?? index.toString() | ||
| return <MappedActionListItem key={key} {...item} renderItem={listProps.renderItem} /> | ||
| })} | ||
| </ActionList.Group> | ||
| ) | ||
| }) | ||
| : items.map((item, index) => { | ||
| const key = item.key ?? item.id?.toString() ?? index.toString() | ||
| : items.map(({key: itemKey, ...item}, index) => { | ||
| const key = itemKey ?? item.id?.toString() ?? index.toString() |
There was a problem hiding this comment.
[nitpick] Verify that destructuring 'itemKey' for generating the key is consistently applied and clearly documented when key fallback values are used.
Copilot uses AI. Check for mistakes.
size-limit report 📦
|
|
👋 Hi from github/github! Your integration PR is ready: https://github.com/github/github/pull/375036 |
|
🟢 golden-jobs completed with status |
github-actions
bot
added
integration-tests: passing
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
Realized when going through our eslint setup that we're not using the new jsx transform when building and testing our components. For more information, see: https://legacy.reactjs.org/blog/2020/09/22/introducing-the-new-jsx-transform.html
Changelog
New
Changed
automatictransform instead ofclassicwhich is defaultkeywas not being explicitly passed to a componentRemoved
Rollout strategy