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

Expired cookie set to "undefined", not undefined #37

Closed
jamesplease opened this issue Jan 25, 2015 · 3 comments
Closed

Expired cookie set to "undefined", not undefined #37

jamesplease opened this issue Jan 25, 2015 · 3 comments
Labels

Comments

@jamesplease
Copy link

Type: Bug

Description: Expiring a cookie sets its value as a string, "undefined".

What I would expect: The value would be the 'default' value for cookies, or, undefined. This is important for libraries that check for the existence of a cookie by its value, which is important given that there is no other method to check for existence.

Cause: I suspect that this is what happens: Cookies.expire delegates to Cookies.set, passing undefined as the value. That method then delegates to _generateCookieString to compute the actual value to set. L88, within that method, is the likely culprit. undefined + "" gets converted to "undefined".

If you're interested in a fix, I can whip up a PR.

Temporary Fix: Fix the value of any cookie in your code after retrieving it.

var cookie = Cookies.get('cookieName');
if (cookie === 'undefined') { cookie = undefined; }
@jamesplease
Copy link
Author

nevermind, this works fine in the browser. It seems to be an issue with JSDom.

@ScottHamper
Copy link
Owner

Interesting issue - it sounds like JS Dom is ignoring the "expires" option that gets set.

In reality, Cookies.js does set the cookie value to "undefined" - but it also sets "expires" to be one second in the past. Setting the date in the past causes browsers to immediately delete the cookie, so it no longer matters what the cookie value is. JS Dom just might not be respecting this expiration mechanic.

@jamesplease
Copy link
Author

JS Dom just might not be respecting this expiration mechanic.

That could very well be the case! This issue is likely related to the discussion here, jsdom/jsdom#310, where it sounds like cookies are still being worked on in JSDom.

I don't have the time to investigate it further, so (for the record), I'm getting around it by including following snippet in my node-only test setup:

var cookies = require('cookies-js');
var originalGet = cookies.get;
cookies.get = function(cookieName) {
  var value = originalGet(cookieName);
  return value === 'undefined' ? undefined : value;
};

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants