Skip to content

Conversation

zachasme
Copy link

@zachasme zachasme commented Sep 11, 2019

Resolves #101.

This enforces string input as suggested by @dougwilson.

Two considerations:

  • Is input a string if wrapped in String constructor? I suggest no, as do Node. That is also what I went with in the PR.
  • When is input omitted? Logically undefined. But there are tests for expiring cookies using null. What about false? I feel undefined and null should cover it (an explicit Cookies.remove would avoid ambiguity, see Add cookies.remove(name[, options]) method #98).

@dougwilson dougwilson added the pr label Oct 9, 2019
@dougwilson
Copy link
Contributor

Is input a string if wrapped in String constructor? I suggest no, as do Node. That is also what I went with in the PR.

Yea, I have just done the same type of validations that Node.js APIs use, which is just typeof, only considering primitive types and not the boxed types.

When is input omitted? Logically undefined. But there are tests for expiring cookies using null. What about false? I feel undefined and null should cover it

From a JavaScript POV, it is just undefined. The two tests passing in null, are testing that any falsy value works, so that would mean the API contract there does indeed false and 0.

@zachasme
Copy link
Author

Would you prefer only considering input as omitted on undefined then? If so I can quickly update the PR accordingly.

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

Successfully merging this pull request may close these issues.

Cookie set to '' when pass the value as 0
2 participants