-
Notifications
You must be signed in to change notification settings - Fork 223
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
Upgrade cookie to 0.5.0 #92
Conversation
SameSite set to none is valid
|
Hey @MygengBin From the changelog I don't think there is any change to SameSite attribute within this version so the issue may be unrelated.
SameSite=None support was added in 0.4.0 Make sure your format is correct when sending this:
Here is the reference to code where |
@dougwilson Would you be able to please review this, whenever you can spare some time? |
Hello 👋! Yes, the cookie module can be bumped, though I don't think it will help woth the samesite problem. I can do it, or if you would like me to merge this PR, just need the commut message to match the other bumps ans also need to add to history.md the changes too. |
i am look from cookie-parser, found not that question, but i seen this error path in express , i usually |
d26d176
to
1571e78
Compare
Thank you for the response @dougwilson 🙌🏼 - the intention behind raising this PR was as mentioned in the description & not really related to SameSite attribute, like I highlighted earlier the behaviour for SameSite hasn't changed in this version bump.
Referring to an older commit: 695435a I've made changes to change the commit message, as well as introduced a Note: I've made a Let me know if this looks good, open to further suggestions. |
A gentle reminder on the review for the suggested changes, whenever you can find the time :) - @dougwilson |
af71e39
to
b788193
Compare
@dougwilson Hey! Just checking back on this, in the hopes of getting this merged |
b788193
to
5a0e0ca
Compare
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.
LGTM! This should be merged and released before #105
Hello! This would help us as well, can we please get this merged and released? |
surpassed #116 |
Hey! Noticed that the package
cookie
that we depend on has the latest version available that we can upgrade on which has a few perf benefits.This although primarily comes from us using
cookie-parser
in our project which inturns is using an outdated version of thecookie
package, while we're on the latest version & not able to de-dupe this to resolve to a single version leading to duplicate versions coming as part of the bundle.Here's the changelog:
https://github.com/jshttp/cookie/releases
Open to suggestions.