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

remove lazy_static #373

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

ValiuchenkoVladyslav
Copy link

Goal

Remove lazy_static dependency in favor of std::sync::LazyLock

Checklist

  • I have followed the steps listed in the Contributing guide.
  • If necessary, I have added documentation related to the changes made.
  • I have added or updated the tests related to the changes made. (no changes required)

@gustavo-shigueo
Copy link
Collaborator

gustavo-shigueo commented Dec 5, 2024

Hello @ValiuchenkoVladyslav, thanks for the PR! If I'm not mistaken, this change would raise the crate's MSRV right? Could you update the README and the Cargo.toml to reflect that change? Make sure to follow the instructions in CONTRIBUTING.md for README updates

@ValiuchenkoVladyslav
Copy link
Author

Thanks for pointing out. My change indeed raises MSRV up to 1.80.0 according to docs https://doc.rust-lang.org/beta/std/sync/struct.LazyLock.html

@gustavo-shigueo
Copy link
Collaborator

Awesome, I'll label the PR as a breaking change due to the bump to the MSRV, other than that, as long as @NyxCode approves it, we can merge

@gustavo-shigueo gustavo-shigueo added the breaking change This PR will change the public API in some way label Dec 5, 2024
@ValiuchenkoVladyslav
Copy link
Author

After reading docs again, I stumbled across OnceLock which acts similar to LazyLock but came to std 10 minor versions earlier (1.70.0) so I made changes and decided to test msrv with cargo msrv find --min 1.63.0 (https://github.com/foresterre/cargo-msrv) just to be sure.

It produced unexpected results so I decided to test it on current main branch which produced errors too (1.63.0 as stated in README; targeting x86_64-pc-windows-msvc):

error[E0445]: crate-private trait `attr::Attr` in public interface
   --> macros\src\utils.rs:114:1
    |
114 | / pub fn parse_serde_attrs<'a, A>(attrs: &'a [Attribute]) -> Serde<A>
115 | | where
116 | |     A: Attr,
117 | |     Serde<A>: TryFrom<&'a Attribute, Error = Error>,
    | |____________________________________________________^ can't leak crate-private trait
    |
   ::: macros\src\attr\mod.rs:30:1
    |
30  |   pub(super) trait Attr: Default {
    |   ------------------------------ `attr::Attr` declared as crate-private

error[E0446]: crate-private type `Serde<A>` in public interface
   --> macros\src\utils.rs:114:1
    |
114 | / pub fn parse_serde_attrs<'a, A>(attrs: &'a [Attribute]) -> Serde<A>
115 | | where
116 | |     A: Attr,
117 | |     Serde<A>: TryFrom<&'a Attribute, Error = Error>,
    | |____________________________________________________^ can't leak crate-private type
    |
   ::: macros\src\attr\mod.rs:42:1
    |
42  | / pub(super) struct Serde<T>(pub T)
43  | | where
44  | |     T: Attr;
    | |____________- `Serde<A>` declared as crate-private

Some errors have detailed explanations: E0445, E0446.
For more information about an error, try `rustc --explain E0445`.
error: could not compile `ts-rs-macros` due to 2 previous errors

I also tried to test it using rust-toolchain file (same results)

[toolchain]
channel = "1.63.0"
components = []

Btw cargo-msrv produces 1.74.1 for x86_64-pc-windows-msvc so we can remove lazy_static using OnceLock without bumping the actual MSRV

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking change This PR will change the public API in some way
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants