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

Only partial re-export of cookie types? #23

Closed
ilyvion opened this issue Jan 21, 2023 · 6 comments · Fixed by #24
Closed

Only partial re-export of cookie types? #23

ilyvion opened this issue Jan 21, 2023 · 6 comments · Fixed by #24

Comments

@ilyvion
Copy link
Contributor

ilyvion commented Jan 21, 2023

So I noticed that there are some pub use cookie::<Whatever>s in tower-cookies, but it only covers some of the types. Like if you try to do

use tower_cookies::{Cookie, Cookies};

Cookie::build(...)
    .same_site(SameSite::Lax)

you are told

error[E0433]: failed to resolve: use of undeclared type `SameSite`
   --> path/to/file.rs:103:24
    |
103 |             .same_site(SameSite::Lax)
    |                        ^^^^^^^^ use of undeclared type `SameSite`

and there's no SameSite to import from tower_cookies. Obviously I could just add a dependency on cookie and import it from there, but I feel like this library would be less confusing if it either exported all of cookie (or at least the parts that are part of the cookie creation API) or none of cookie; exporting only parts makes the API surface only partially accessible "by default."

@imbolc
Copy link
Owner

imbolc commented Jan 21, 2023

Thanks, that makes sense, would you provide a PR?

@ilyvion
Copy link
Contributor Author

ilyvion commented Jan 22, 2023

I'll see what I can do. 😄👍

@ilyvion
Copy link
Contributor Author

ilyvion commented Jan 22, 2023

Er, which option do you prefer? Exporting "everything" or nothing?

@imbolc
Copy link
Owner

imbolc commented Jan 22, 2023

Maybe we leave current reexports as they are to avoid api breaking and just add pub use cookie? What do you think?

@ilyvion
Copy link
Contributor Author

ilyvion commented Jan 22, 2023

That seems decent enough, yeah. 👍

ilyvion added a commit to ilyvion-contrib/tower-cookies that referenced this issue Jan 23, 2023
@imbolc imbolc closed this as completed in 5403725 Jan 23, 2023
@imbolc
Copy link
Owner

imbolc commented Jan 23, 2023

It's published, thank you 🙏

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 a pull request may close this issue.

2 participants