voltace / browser-cookies

Tiny cookies library for the browser
The Unlicense
89 stars 18 forks source link

When no name is passed, get all cookies as an object #6

Closed wesleytodd closed 7 years ago

wesleytodd commented 7 years ago

Use case:

I need to remove an unknown set of cookies and want to leverage this module's cookie parsing instead of parsing it myself and then calling cookies.erase for each. This would allow me to do the following:

var toKeep = ['foo', 'bar'];
var c = cookies.get();
Object.keys(c).filter(function (name) {
  return toKeep.indexOf(name) === -1;
}).forEach(function (name) {
  cookies.erase(name);
});

Resulting in just foo and bar remaining. Make sense?

wesleytodd commented 7 years ago

The test failure seems to be a broken integration with sauce labs, not because of this addition.

voltace commented 7 years ago

It's indeed not possible to remove unknown cookies using the current API. I agree it makes sense allow retrieval of all cookies to support these kind of advanced use cases. Perhaps it's even justified to introduce a new dedicated method such as cookies.all().

Due to security reasons Sauce Labs is not allowed on pull requests, but your test code passed the local (PhantomJS) test on travis so it seems fine. I'll make sure to add some bad weather test cases such as handling of falsey cookies names.

Note that the current framework that's used to run the unit tests on Sauce Labs has gotten unreliable, so this is something I will need to fix first.

wesleytodd commented 7 years ago

I am fine with changing it to cookies.all() if that is what you would rather have. Not sure if one is objectivly better than the other, but its your lib :)

Let me know what you would prefer and I can update the PR.

voltace commented 7 years ago

I like the cleverness of returning all cookies when no name is specified, however after some thought I've decided I prefer using cookies.all() because it's more explicit which improves code readability and eases referencing to this new functionality in the documentation/examples sections.

wesleytodd commented 7 years ago

Changes made

wesleytodd commented 7 years ago

Do you have a estimate for when you will be ready to publish these changes to npm?

voltace commented 7 years ago

I shall publish the new release (v1.1.0) to npm this weekend