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

feat: remove read signedCookies and cookies #1026

Open
wants to merge 1 commit into
base: v2
Choose a base branch
from

Conversation

bjohansebas
Copy link
Member

As part of removing deprecated features and dropping compatibility with cookie-parse

Copy link

@raphendyr raphendyr left a comment

Choose a reason for hiding this comment

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

I find this good and clean remove.

Going forward, would there be benefits to eventually remove cookie-parser dev dependency? At the time of this PR, I value these tests though.

@raphendyr
Copy link

Side note. As these other cookie libraries are removed, then this feature request is not going to be implemented either, I think: #1.

@bjohansebas
Copy link
Member Author

Going forward, would there be benefits to eventually remove cookie-parser dev dependency? At the time of this PR, I value these tests though.

Yep, at some point, maybe for version 3. I’m keeping the library to make sure it's working well with that dependency. Maybe at some point, when we have the examples repository, we can move those tests there.

As these other cookie libraries are removed, then this feature request is not going to be implemented either, I think: #1.

I would like to remove the handling of cookies from this library and leave it to cookies, that's why I reopened it.

@raphendyr
Copy link

As these other cookie libraries are removed, then this feature request is not going to be implemented either, I think: #1.

I would like to remove the handling of cookies from this library and leave it to cookies, that's why I reopened it.

I think I misunderstood the issue last time I read it. Now, it makes total sense. Yeah, cookie prosessing should be handled by some other library.

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.

3 participants