Skip to content

Conversation

@eps1lon
Copy link
Contributor

@eps1lon eps1lon commented Jun 24, 2024

This will help propagation of bug fixes or features (e.g. React 19 support).

If Radix UI frequently breaks semantic versioning, it should be avoided. Pinning packages to patch versions contributes to bundle size bloat.

@vercel
Copy link

vercel bot commented Jun 24, 2024

@eps1lon is attempting to deploy a commit to the resend Team on Vercel.

A member of the Team first needs to authorize it.

This will help propagation of bug fixes or features (e.g. React 19 support).

If Radix UI frequently breaks semantic versioning, it should be avoided.
Pinning packages to patch versions contributes to bundle size bloat.
@eps1lon eps1lon force-pushed the radix-caret-range branch from 6c0c6a1 to 681d265 Compare June 24, 2024 17:48
@eps1lon eps1lon marked this pull request as ready for review June 24, 2024 17:57
@gabrielmfern
Copy link
Member

I'd prefer it pinned for now. But still, upgrading them to properly support React 19 is a must, #1474 already is meant to do that, but I'd be willing to merge this and release before we actually have #1474.

Could you just have them pinned?

@eps1lon
Copy link
Contributor Author

eps1lon commented Jun 24, 2024

I'd prefer it pinned for now.

Could you elaborate why that is?

Could you just have them pinned?

I'm not following. If the package is pinning them, it's pinned in every app unless people apply overrides to their dependency resolution. But since this is an additional step, that requires additional knowledge, it's seldom applied so most apps end up with multiple package versions.

@gabrielmfern
Copy link
Member

gabrielmfern commented Jun 24, 2024

Could you elaborate why that is?

Changing this would be better with unpinning other dependencies as well, currently we have as convention to keep them pinned. We might discuss it later on and consider if we want to change that, but just unpinning a set of dependencies and keeping others pinned might be a bit confusing. Appreciate your arguments on this, though, we will keep them in mind when we discuss this.

The only exception is that we don't pin React because it causes issues, and because we want to support multiple React versions.

I'm not following.

I meant it as asking if you could change the versions in the PR to be pinned 😅

@eps1lon eps1lon changed the title feat(react-email): Relax Radix-UI React dependency to 1.x feat(react-email): Bump Radix-UI React Jun 25, 2024
@eps1lon
Copy link
Contributor Author

eps1lon commented Jun 25, 2024

I meant it as asking if you could change the versions in the PR to be pinned 😅

Got it. Pinned to patch now.

Copy link
Member

@gabrielmfern gabrielmfern left a comment

Choose a reason for hiding this comment

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

Thanks for this! Highly appreciate it, we'll also keep in mind what you mentioned about bundle size.

@gabrielmfern gabrielmfern merged commit cf5a6f4 into resend:canary Jun 25, 2024
@eps1lon eps1lon deleted the radix-caret-range branch June 25, 2024 19:26
@eps1lon eps1lon mentioned this pull request Jun 27, 2024
22 tasks
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.

2 participants