Skip to content

enh: hygienize usages of re-exports #4069

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

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

Conversation

mondeja
Copy link
Contributor

@mondeja mondeja commented Jun 12, 2025

The crate leptos re-exports several dependencies to use them in the #[component] macro:

  • wasm_bindgen
  • web_sys - There is a direct usage of it in an example.
  • tracing
  • serde
  • serde_json

This is problematic because it could hide the definition of direct dependencies of a project in Leptos, which can cause hard to debug issues with version mismatches of duplicated dependencies and duplication of dependencies itself. Additionally, it confuses to developer consumers of Leptos about if they must use, for some reason, the dependencies from this path.

Leptos has an example with this problem going unnoticed that serves to understand the problem.

  1. Comment the line use leptos::wasm_bindgen::JsCast; in examples/directives/src/lib.rs.
  2. Run trunk serve in the examples/directives directory.

Rustc will show an error like this:

error[E0599]: no method named `unchecked_into` found for struct `Element` in the current scope
  --> src/lib.rs:54:17
   |
54 |     let el = el.unchecked_into::<web_sys::HtmlElement>();
   |                 ^^^^^^^^^^^^^^
   |
  ::: /.../.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/wasm-bindgen-0.2.100/src/cast.rs:78:8
   |
78 |     fn unchecked_into<T>(self) -> T
   |        -------------- the method is available for `Element` here
   |
   = help: items from traits can only be used if the trait is in scope
help: trait `JsCast` which provides `unchecked_into` is implemented but not in scope; perhaps you want to import it
   |
1  + use leptos::wasm_bindgen::JsCast;
   |
help: there is a method `unchecked_into_f64` with a similar name
   |
54 |     let el = el.unchecked_into_f64::<web_sys::HtmlElement>();
   |                               ++++

Note that the error message suggests to import JsCast from leptos::wasm_bindgen, which is not a good suggestion for the reasons explained above.

The solution included in this PR is to hide re-exports of leptos in a __reexports internal public module. This is the solution suggested in rust-lang/rust#63687 (comment) for hygienic usage of libraries in proc-macro generated code.

There is a pattern to use std or any other third party crate in a hygienic way. You define a #[doc(hidden)] pub mod __reexports { pub use std; } in lib.rs and then use $crate::__reexports::std::... to refer to std items in the macro.

You can see linked PRs in the issue of the linked comment examples of the same solution in other libraries. Are called different to __reexports, sometimes __export (liballoc), __macro_support... but it is consistent that rustc will not suggest to import from a module starting with __.

@gbj
Copy link
Collaborator

gbj commented Jun 12, 2025

This is also a breaking change to existing user code that uses the re-exports, so that is an important trade-off to consider in removing the re-exports, rather than making a decision about what would be ideal if we were starting from scratch.

@mondeja
Copy link
Contributor Author

mondeja commented Jun 12, 2025

This is also a breaking change to existing user code that uses the re-exports, so that is an important trade-off to consider in removing the re-exports, rather than making a decision about what would be ideal if we were starting from scratch.

So it's a breaking change, I'm OK with it.

@gbj
Copy link
Collaborator

gbj commented Jun 12, 2025

Maybe you could expand on the reasoning in favor of the change, then? I don't understand either of the problems you describe.

Specifically:

This is problematic because it could hide the definition of direct dependencies of a project in Leptos, which can cause hard to debug issues with version mismatches of duplicated dependencies

Of the 5 you list here (serde, serde_json, tracing, web-sys, wasm-bindgen) none have had a new major version/semver-breaking release in many years, as far as I know. Are you thinking of a particular example in which there has been a version mismatch with one of these crates? They are very stable. Worst case, upgrading a major version of one of them would require a version bump. (Like what happens when axum bumps its version and we need to release a new leptos_axum version.)

and duplication of dependencies itself.

cargo will deduplicate the dependencies unless there are two different major versions required (which hasn't happened in the framework's history -- see above)

Additionally, it confuses to developer consumers of Leptos about if they must use, for some reason, the dependencies from this path.

I'm not sure I understand the confusion. In the example you give, you remove the line use leptos::wasm_bindgen::JsCast; and the compiler suggests you add the line use leptos::wasm_bindgen::JsCast; This seems pretty straightforward to me. (In fact, it seems better to me than requiring a user to cargo add wasm_bindgen in order to do something that's relatively common, but maybe that's a matter of opinion.)


If I were creating the library again in a vacuum, would it be worth #[doc(hidden)] pub mod __reexports { pub use std; }? Maybe! But I don't understand why they are bad enough that we should remove them, when they've all been reexported since 0.1.

@mondeja
Copy link
Contributor Author

mondeja commented Jun 12, 2025

This seems pretty straightforward to me. (In fact, it seems better to me than requiring a user to cargo add wasm_bindgen in order to do something that's relatively common, but maybe that's a matter of opinion.)

That is a matter of opinion, it's the whole point of this PR.

If the framework wants to bundle in their public API wasm_bindgen or web_sys, like web_sys itself does with wasm_bindgen and js_sys, it should be documented or we'll end increasing the maintainment efforts for things so much naive like knowing what dependency is using an app. Especially for locked installs of apps that are pinning to an exact version of these dependencies, which is not possible to do so if someone is using a dependency like wasm_bindgen from leptos in a third party library.

Are you thinking of a particular example in which there has been a version mismatch with one of these crates? They are very stable.

Yes, the web is plenty of cases where wasm-bindgen has been pinned to a specific version in Cargo.toml files to solve a problem because the bidgen format is unstable and all releases are like major version changes. See https://stackoverflow.com/questions/76452749/leptos-with-axum-issue-with-wasm-bindgen-versions If wasm-bindgen is used in an application from leptos and as direct dependency when leptos is pinning it to ^0.2.100 currently, how could will not end the bundle with two duplicated versions?

I don't really care so much. These dependencies are not there because is easier to import from leptos itself than installing them properly. Are there because were needed to be added for generating proc-macro code in an unhygienic way. If you're fine with keeping them reexported in that way, I'm OK, and will write a lint to forbid leptos reexports because I don't want to deal with these maintainment nightmares. I was going to write the lint at first, just opened this in case it could be solved in a better way. Feel free to close.

@mondeja
Copy link
Contributor Author

mondeja commented Jun 25, 2025

For the people interested on this, I've written a lint to forbid usages of these re-exports from leptos. See leptos_reexports of mondeja/rs-apps-lints and simple-icons/simple-icons-website-rs#381

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