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

fix: Omit props removed in future versions of @types/react #819

Merged
merged 2 commits into from
Mar 29, 2024

Conversation

pawelgrimm
Copy link
Contributor

@pawelgrimm pawelgrimm commented Mar 27, 2024

Short description

In newer versions of the @types/react package, certain props on certain components have been removed. When consumers of Reactist upgrade that package, they will run into type errors stating that there are missing props on a bunch of Reactist components. For example, the Button component would require the placeholder, onPointerEnterCapture, and onPointerLeaveCapture props to be specified on all usages of that component.

Eventually, we should upgrade the @types/react package in this repository as well, but this is blocked by upgrading to the latest @ariakit/react version. In the interim, the fix in this PR should allow consumers to continue using this package without issues.

PR Checklist

  • Added tests for bugs / new features
  • Updated docs (storybooks, readme)
  • Executed npm run validate and made sure no errors / warnings were shown
  • Described changes in CHANGELOG.md
  • Bumped version in package.json and package-lock.json (npm --no-git-tag-version version <major|minor|patch>) ref
  • Reviewed and approved Chromatic visual regression tests in CI

Versioning

Breaking change (major)

@gnapse
Copy link
Contributor

gnapse commented Mar 28, 2024

Should this PR be made on top of next instead? Or at the very least, do it there as well? That's the branch that we're currently consuming in todoist-web.

@pawelgrimm
Copy link
Contributor Author

Should this PR be made on top of next instead? Or at the very least, do it there as well? That's the branch that we're currently consuming in todoist-web.

@gnapse That's a great question. What's your plan for releasing the current beta version (v24.1.2-beta) of Reactist? Should I wait for that to happen before I cut this into a patch release? Or should I merge this in and release it as v24.1.3-beta?

@gnapse
Copy link
Contributor

gnapse commented Mar 28, 2024

What's your plan for releasing the current beta version (v24.1.2-beta) of Reactist?

I first want to address the issue about the as="…" prop. And unfortunately, I haven't been able to make time for it this week.

Should I wait for that to happen before I cut this into a patch release? Or should I merge this in and release it as v24.1.3-beta?

No, let's not wait. Let's put this in a new beta release.

@pawelgrimm
Copy link
Contributor Author

@gnapse I'm a bit torn on the type of version bump to use here. Technically, it's an API change, since we're changing the props we accept for a bunch of components. On the other hand, this is purely a type-level change and we're still passing those props forward, though it's unclear if those props were ever actually supported. Do you agree that it should still be a new major version though?

@pawelgrimm pawelgrimm force-pushed the pawel/fix/react-types-workaround branch from f34ef54 to 847b6e9 Compare March 28, 2024 17:05
@pawelgrimm pawelgrimm changed the base branch from main to next March 28, 2024 17:06
@gnapse
Copy link
Contributor

gnapse commented Mar 28, 2024

I'm as torn as you, but with the recommendation to add it to the beta v24 (without increasing it to v25). v24 is set out to be the next major release (once we get past the issue with the as="…" prop). So it is the perfect opportunity to release this change, also as part of the breaking changes that prompt the jump from v23 to v24. Makes sense?

@pawelgrimm pawelgrimm force-pushed the pawel/fix/react-types-workaround branch from 847b6e9 to a3335ef Compare March 29, 2024 15:30
@pawelgrimm pawelgrimm merged commit e13270f into next Mar 29, 2024
2 checks passed
@pawelgrimm pawelgrimm deleted the pawel/fix/react-types-workaround branch March 29, 2024 15:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants