`Options` params usage in `constructor` and `set`
Closed this issue · 2 comments
Hi @will-stone, thanks for the great work providing a reactive cookie for mobx
with mobx-cookie
.
Recently, I ran through the code as I need to provide a Typescript definition for this library (we have to use Typescript at work). And there is a minor thing I reckon we can do.
class MobxCookie {
/**
* @param {*} name cookie's name
* @param {*} options (optional) options to send to js-cookie
*/
constructor(name, options) {
this.name = name // name of set cookie
this.options = options // options to send to js-cookie
this._timeout = null // internal timer
extendObservable(this, {
value: jsCookie.get(name) // observable to keep in-sync with cookie value
})
this._syncTimeout()
}
Our constructor is currently accepting a second parameter as js-cookie
options. However, I don't think we use this.options
anywhere, instead, if someone want to set the option, he/she needs to do it everytime set
function is called.
I reckon there are two separated ways we can improve this:
1.) Drop options
as the second parameter of the constructor.
2.) set
function will make use of this.options
if no options
param is passed to this function, if there is an options
param passed, it will be merged and overriden the class options
.
What do you think ? I am happy to submit a PR for this issue.
It looks like I removed the last use of this.options
here: c5f9a60#diff-a27cdde3e6123d1f6fd9a1279fa39eceL50 I have no idea why I needed it 😄 That's another thing that needs adding to the project: tests!
Now you've got me thinking about this project, I do feel the want to improve it. Let me think about it. I may give it a go this weekend (no promises).
Removed this.options
in #5