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

feat(deps): react and react-dom peer dependencies #1857

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

RubenSibon
Copy link
Contributor

@RubenSibon RubenSibon commented Feb 7, 2025

Describe the pull request

Thank you for contributing to the project!
Please use this template to help us handle your PR smoothly.

What

Increments the peer dependencies for react and react-dom so that the React package supports React 17, 18 and 19.

Why

React 19 has come out recently and React 16 is no longer used much.

How

Changing numbers in the package.json of the react package.

The ADS has been tested with:

You can run above linked projects locally to check it our for yourself.

Checklist

Before submitting your pull request, please ensure you have done the following. Check each checkmark if you have done so or if it wasn't necessary:

  • Add or update documentation
  • Start the PR title with a Conventional Commit prefix, as explained here.

Additional notes

  • See Jira research ticket for internal documentation and considerations.
  • Do we still want to support React 16?
  • Do we want to upgrade ADS own React dependencies to 19.x?

@RubenSibon RubenSibon self-assigned this Feb 7, 2025
@RubenSibon RubenSibon changed the title build(deps): react and react-dom peer dependencies chore(deps): react and react-dom peer dependencies Feb 7, 2025
@RubenSibon RubenSibon marked this pull request as ready for review February 7, 2025 15:44
@RubenSibon RubenSibon requested a review from a team as a code owner February 7, 2025 15:44
@@ -77,7 +77,7 @@
},
"peerDependencies": {
"@amsterdam/design-system-css": "workspace:*",
"react": "16 - 18",
"react-dom": "16 - 18"
"react": "17 - 19",
Copy link
Contributor

Choose a reason for hiding this comment

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

Removing 16 is a breaking change, we shouldn't do that if it isn't necessary.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alright, that would be okay with me.

But have you seen this DES-1130 comment?

There are, presumably, only 3 projects within the Amsterdam GitHub that use React 16. Of those 3, 2 belong to OpenStad. I checked with Ian Rieken, who maintains the OpenStad-stack, and he says those two repo's don't use the ADS anyway and are no longer maintained. Whether makkelijkemarkt-kiesjekraam uses the ADS I do not know.

There could be projects that were not counted or are outside of the Amsterdam GitHub organisation that use the ADS.

@alimpens
Copy link
Contributor

I think this is a feat, it definitely belongs in the changelog. And if we decide to not support React 16 anymore, it's a breaking feature.

@RubenSibon RubenSibon changed the title chore(deps): react and react-dom peer dependencies feat(deps): react and react-dom peer dependencies Feb 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants