Js-cookie: Suggestions

Created on 21 Feb 2020  ·  6Comments  ·  Source: js-cookie/js-cookie

I have just tried out this library and i think it is nice.
I noticed a couple of things that could maybe be improved, and i also have a couple of suggestions.

I think it would be nice to have a 'check if exists' function.
I already had to do this is several projects, and i ended up doing something like:
document.cookie.indexOf('cookieName') != -1
Something like Cookies.exists() would be great.

I got an error in the set function because i was passing the 'expires' value as a string (from a variable) instead of a number.

  • First suggestion here is that it accepts strings (a simple validation and parseInt) ... ;)
  • the second idea is to have an easier to understand error. Something like "Make sure to pass an Integer as a value"...

Other than that it worked well.
Thank you!
=D

feature request

Most helpful comment

If you take the entire sentence it will maybe make more sense:

Value can be a Number which will be interpreted as days from time of creation or a Date instance.

IMO that doesn‘t leave a lot of room for interpretation (either Number or Date), but at the same time I‘m not a native speaker and we could have written: “Value must be a Number ... or ...”.

All 6 comments

I think it would be nice to have a 'check if exists' function.

If we implement an .exists() functionality it will essentially mimic !!Cookies.get(...) only with more code and more tests. Is there anything an exists functionality gives that a !!Cookies.get(...) can't? I'm up to document !!Cookies.get(...) if that's the case.

I got an error in the set function because i was passing the 'expires' value as a string (from a variable) instead of a number.

  • First suggestion here is that it accepts strings (a simple validation and parseInt) ... ;)
  • the second idea is to have an easier to understand error. Something like "Make sure to pass an Integer as a value"...

The library runs in the assumption that developers will read the docs fully before using it to understand what's valid and what's not. See this post that explains the fundamental aspects of the idea behind it. In this case, if you pass a type that is not documented, you're using internal behavior, which should not be done.

I'm up to get feedback and improve the docs to help prevent the mistake of people using undocumented types for the expires.

If we start validating every input, it's like we're encouraging people to not read the docs. If you make mistakes by passing wrong undocumented input, you'll learn over time that reading the docs is important to know how the tool works and the understand intention of the library authors of what can be publicly available. It's like a forcing function to instruct folks in the right direction instead of hiding the mistake under a runtime check.

Thanks for these suggestions...!

As @FagnerMartinsBrack has explained we're covering the first one and will refrain from adding string parsing/support for dates for the time being.

First of all, please take my comments as suggestions, i am not trying to criticizing your library. I think it is great, and that is why i am giving you my feedback. My goal is to provide you with constructive comments from a user's point of view

I think the goal of a library like this is to simplify things and create an API that is easier to use than the native APIs.
I guess something like !!Cookies.get(...) could work, but it feels somewhat "raw" or "hacky".

I agree that users should go through the documentation to understand how a library works. I know first hand how much work it is to create good documentations, and i also know how useful good documentation are! 😉
The thing is that documentations are often outdated or incomplete, so not everyone relies 100% on them.
In this case maybe adding the !!Cookies.get(...) to the documentation wouldn't be a bad thing.
But i also don't see how a new exists() method, that would return !!Cookies.get(...) (or ideally something more robust than that) wouldn't be an improvement to the library.

As to the second point, you can't just not have error handling, expecting that users will use the your library perfectly.
Type checking wouldn't be difficult to implement (many would even suggest it is essential!), nor would throwing a meaningful error.

For instance in the documentation you say _"Value can be a Number which will be interpreted as days"_... More correct would be to say it "must" be a number, because saying it can be a number does not mean it cannot be a String or something else.

Many people that use this kind of libraries don't have a lot of experience in debugging, and meaningful errors are very useful.

Of course you are the library developers and you have the right to develop it as you want. This were just a couple of thoughts i thought could be useful.

If you take the entire sentence it will maybe make more sense:

Value can be a Number which will be interpreted as days from time of creation or a Date instance.

IMO that doesn‘t leave a lot of room for interpretation (either Number or Date), but at the same time I‘m not a native speaker and we could have written: “Value must be a Number ... or ...”.

@luis-pato You have just spotted a bug in the documentation, which is great! Wanna create a PR to change it to Value must be a Number?

Was this page helpful?
0 / 5 - 0 ratings

Related issues

sudhir600 picture sudhir600  ·  9Comments

yaconnn picture yaconnn  ·  6Comments

BonBonSlick picture BonBonSlick  ·  9Comments

DeadcatDev picture DeadcatDev  ·  3Comments

czebe picture czebe  ·  3Comments