electron-userland / electron-json-storage

:package: Easily write and read user settings in Electron apps
1.44k stars 80 forks source link

Optional `options` is no longer optional with v3.2.0 #91

Closed iamchucky closed 7 years ago

iamchucky commented 7 years ago

Due to the changes in v3.2.0, there are bugs when calling storage.set(key, json, [options], callback) without options

Uncaught Exception:
TypeError: Cannot read property 'dataPath' of undefined
    at Object.exports.set

It throws error when trying to access options.dataPath at https://github.com/electron-userland/electron-json-storage/blob/master/lib/storage.js#L256

I think we need to ensure options is set to {} at https://github.com/electron-userland/electron-json-storage/blob/master/lib/storage.js#L251 and make sure all other similar methods do the same thing, like .get and .has .remove .clear

jviotti commented 7 years ago

Hi @iamchucky,

Can you show me an example that reproduces the error? There are various cases on the test suite where we use the .set() function without options (i.e. https://github.com/electron-userland/electron-json-storage/blob/master/tests/storage.spec.js#L198), but maybe I'm missing something?

iamchucky commented 7 years ago

I was not passing callback either:

storage.set('settings', data);

example like above would work before 3.2.0 but throws error with 3.2.0

But now I realize that I should probably pass the callback function in as it is a required argument according to the documentation.

jviotti commented 7 years ago

Ah, that makes sense. I think it should still be easy to fix. I'll prepare a PR later today.

jviotti commented 7 years ago

@iamchucky Can you give https://github.com/electron-userland/electron-json-storage/pull/93 a go?

jviotti commented 7 years ago

Check v4.0.1

iamchucky commented 7 years ago

I don't think #93 fixes this particular issue, when call storage.set('settings', data); without both options and callback, options is undefined, therefore throws at https://github.com/electron-userland/electron-json-storage/blob/8c0519c0ae786c78f590a26d066205d39acf48e7/lib/storage.js#L269

Uncaught Exception:
TypeError: Cannot read property 'dataPath' of undefined
    at Object.exports.set
jviotti commented 7 years ago

Good point. What about v4.0.2?