-
Notifications
You must be signed in to change notification settings - Fork 65
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
Bump ratatui to v0.26.1 #61
Bump ratatui to v0.26.1 #61
Conversation
a203d75
to
6c3d0f7
Compare
@@ -42,10 +42,10 @@ search = ["dep:regex"] | |||
arbitrary = { version = "1", features = ["derive"], optional = true } | |||
crossterm = { package = "crossterm", version = "0.27", optional = true } | |||
crossterm-025 = { package = "crossterm", version = "0.25", optional = true } | |||
ratatui = { version = ">=0.23.0, <1", default-features = false, optional = true } | |||
ratatui = { version = ">=0.26.1, <1", default-features = false, optional = true } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not understanding why this change is necessary. >=0.23.0
means version 0.23.0 or above hence it doesn't prevent you from updating ratatui to 0.26.1 in your local. Am I missing anything?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
>= 0.23.0
means that the resolver will choose 0.26.2 (or the latest released version of Ratatui at any point when dependency resolution is triggered). It doesn't mean that it will choose the maximum compatible with other dependencies also specified.
This causes pain when an application specifies any earlier version of ratatui (e.g. 0.25.0). While tui-textarea does indeed actually support any version of ratatui up to that point, it would be better to instead specify the caret form: "0.26.2" in the dependencies here (or 0.26.0 as that will have the same effect and not prevent apps upgrading to a later compatible release).
ratatui = { version = ">=0.26.1, <1", default-features = false, optional = true } | |
ratatui = { version = "0.26.2", default-features = false, optional = true } |
This pain point is generally discovered by application users attempting to install an application without the --locked
flag, and several instances can be found in repos that use this crate. (I can link to these if you need examples).
This problem is described in more detail in the cargo book.
- https://doc.rust-lang.org/cargo/reference/resolver.html#version-incompatibility-hazards
- https://doc.rust-lang.org/cargo/reference/resolver.html#recommendations
- https://doc.rust-lang.org/cargo/reference/resolver.html#unexpected-dependency-duplication
The obvious downside to this is that it requires tui-textarea to be reasonably responsive to Ratatui major version bumps (e.g. 0.27.0) in order for apps to be able to upgrade. For this I'd suggest enabling dependabot and adding 1-2 extra maintainers to help triage / apply PRs when you're not available. (#65)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To preface, I'm no expert on dependency resolution, I may be wrong here.
Ratatui is still v0.x so every increase of x
in the version number may contain breaking changes (see the changes in this PR). Because of this, Cargo would include multiple versions of ratatui in your project which means that tui-textarea
uses v0.23 types while your crate uses ratatui v0.26 types. Essentially, your build will break when you use any version above 0.23 because the types no longer match up. I think the usage of >=
is only meant for ranges where there will be no breaking changes.
In the linked issue @joshka also listed the following items from Cargo's documentation that seem to discourage the use of >=
:
EDIT: it seems that while I was typing, joshka already posted their own reply :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
To preface, I'm no expert on dependency resolution, I may be wrong here.
Me neither, but I've seen this specific failure mode on enough Ratatui apps to spend a bit of time understanding why it causes failures. Choosing either 0.26.0 or 0.26.2 and not using >=
is the most appropriate choice here for the benefit of developers of apps using this lib.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see. Thank you both for your explanations. I misunderstood the specification. I don't have time to work on this immediately. But I'll try to look into this some point in this week.
- Updates termion and termwiz - Fixes issue where tui-textarea picks up the wrong versions of packages https://github.com/rhysd/tui-textarea/pull/61\#discussion_r1597672569 - Replaces rhysd#61
Closing in favor of #69. |
Resolves #60