Skip to content

Conversation

@rebeccaalpert
Copy link
Member

@rebeccaalpert rebeccaalpert commented Apr 23, 2025

  1. Dropped React imports and added peer dep for 19
  2. Ran npx codemod react/update-react-imports --target ./packages/module
  3. Fixed TS build errors
  4. Ran npx prettier --write packages/module
  5. Manually fixed example imports since the codemod didn't get them

Testing:

  • Tried pulling in @patternfly/documentation-framework@"6.9.11" and updating PF deps to the pre-release versions in that version, but then other things have problems because they don't have access to the pre-releases:
ralpert@ralpert-mac virtual-assistant % npm install        
npm error code ERESOLVE
npm error ERESOLVE could not resolve
npm error
npm error While resolving: @patternfly/chatbot-root@0.0.0
npm error Found: @patternfly/react-core@6.2.0
npm error node_modules/@patternfly/react-core
npm error   @patternfly/react-core@"^6.2.0" from @patternfly/react-code-editor@6.2.0
npm error   node_modules/@patternfly/react-code-editor
npm error   @patternfly/react-core@"^6.2.0" from @patternfly/react-table@6.2.0
npm error   node_modules/@patternfly/react-table
npm error     dev @patternfly/react-table@"6.3.0-prerelease.1" from the root project
npm error
npm error Could not resolve dependency:
npm error dev @patternfly/documentation-framework@"6.9.11" from the root project
npm error
npm error Conflicting peer dependency: @patternfly/react-core@6.3.0-prerelease.1
npm error node_modules/@patternfly/react-core
npm error   peer @patternfly/react-core@"6.3.0-prerelease.1" from @patternfly/documentation-framework@6.9.11
npm error   node_modules/@patternfly/documentation-framework
npm error     dev @patternfly/documentation-framework@"6.9.11" from the root project
npm error
npm error Fix the upstream dependency conflict, or retry
npm error this command with --force or --legacy-peer-deps
npm error to accept an incorrect (and potentially broken) dependency resolution.
npm error
npm error
npm error For a full report see:
npm error /Users/ralpert/.npm/_logs/2025-04-28T19_28_38_184Z-eresolve-report.txt
npm error A complete log of this run can be found in: /Users/ralpert/.npm/_logs/2025-04-28T19_28_38_184Z-debug-0.log
  • Talked to Nicole and she suggested that I can test this in the demo we used for usability testing - I should be able to pull in React 19 there and avoid the circular dependencies in theory, but I would need this to merge first.

@patternfly-build
Copy link

patternfly-build commented Apr 23, 2025

@rebeccaalpert rebeccaalpert force-pushed the react-19 branch 10 times, most recently from a8363d5 to 29be254 Compare April 28, 2025 18:41
@rebeccaalpert rebeccaalpert linked an issue Apr 28, 2025 that may be closed by this pull request
@rebeccaalpert rebeccaalpert changed the title Draft: chore(React19): Enable React 19 chore(React19): Enable React 19 Apr 28, 2025
@rebeccaalpert
Copy link
Member Author

Just rebased.

@rebeccaalpert rebeccaalpert requested review from a team and removed request for a team May 1, 2025 14:42
@rebeccaalpert
Copy link
Member Author

Rebased.

@rebeccaalpert rebeccaalpert requested a review from nicolethoen May 6, 2025 16:09
Copy link
Contributor

@kmcfaul kmcfaul left a comment

Choose a reason for hiding this comment

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

I'm a little confused about the MouseEvent params (previously we specified React.MouseEvent and native MouseEvent, and with the codemods now it only uses react's import of MouseEvent, losing the native one). Not sure if we still need to cover both explicitly or not.

Another general note is that I'm wondering if we want to keep the React.X notation on prop definitions for our docs. I don't think the build complains about having it there (still leaving out the overall react import), and I've noticed we still have it in the react-core's props. It may not be a big deal to leave this as is with the types imported, as it's usually pretty evident it's a react type for things like ReactNode.

@rebeccaalpert
Copy link
Member Author

Addressed comments and rebased!

Comment on lines -60 to -61
"react": "^18.2.0",
"react-dom": "^18.2.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need to remove these from the devDeps?

Copy link
Member Author

Choose a reason for hiding this comment

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

Katie asked me to put them back in in the other package.json...do we need both?

@rebeccaalpert rebeccaalpert merged commit 76e4911 into patternfly:main May 7, 2025
5 checks passed
@github-actions
Copy link

🎉 This PR is included in version 6.3.0-prerelease.14 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Refactor React imports

4 participants