close
The Wayback Machine - https://web.archive.org/web/20201024065442/https://github.com/sindresorhus/electron-store/issues/52
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Convenience methods #52

Open
sindresorhus opened this issue Oct 31, 2018 · 7 comments
Open

Convenience methods #52

sindresorhus opened this issue Oct 31, 2018 · 7 comments
Labels

Comments

@sindresorhus
Copy link
Owner

@sindresorhus sindresorhus commented Oct 31, 2018

I propose adding some convenience methods to make common things easier.

What I do most of the time when using electron-store is to get a boolean and invert it. For example, store.set('isFoo', !store.get('isFoo')).

Would be nice to have:

.toggleBoolean(key) which accepts a key and toggles the boolean.

.appendToArray(key, newItem) which pushes a new item to the array. (#32)

.modify(key, mutateCallback) which would give you a callback to modify the value that you then return in the callback:

Before:

const array = store.get('array');
array.find(element => element.title === 'baz').src = 'something';
store.set('array', array);

After:

store.modify('array', array => {
    array.find(element => element.title === 'baz').src = 'something';
    return array;
});

These methods will require you to specify the key in the defaults option, so it would always work. If you try to use toggleBoolean() and the underlaying value is of a different type, it would throw an error.

Any other convenience methods we could add? I'm also interested in feedback on the method names.


*Note: While I opened the issue here, the methods we eventually go for should be added to conf.

@austenc
Copy link

@austenc austenc commented Nov 2, 2018

New to using the library and I just tried to push stuff to an array, this would definitely be great to have! The toggleBoolean method would also be very useful!

@ghost
Copy link

@ghost ghost commented Nov 2, 2018

I disagree about adding any convenience methods at all, as it breaks the SRP. toggleBoolean, incrementNumber, appendToArray, prependToArray, insertIntoArray or any other methods do not relate to storing data in electron, but they increase complexity and pollute the API. The only convenience method that makes sense is modify or patch; but, just like set, it should not return the modified value.

@sindresorhus
Copy link
Owner Author

@sindresorhus sindresorhus commented Nov 2, 2018

@asmironov SRP?

@ghost
Copy link

@ghost ghost commented Nov 6, 2018

@sindresorhus, well, the single responsibility principle. From the SOLID.

Electron storing arbitrary data in the json file is one thing, plain and simple. Modifying the data in that file, depending on its structure, is another. It's better not to mix them together.

@y4ure
Copy link

@y4ure y4ure commented Nov 9, 2018

Will be great to have .modify(key, mutateCallback). I think it's the only one that is needed though, .toggleBoolean(key), .appendToArray(key, newItem) etc just looks a bit weird to me.

@cliffordfajardo
Copy link

@cliffordfajardo cliffordfajardo commented Nov 24, 2018

I like the the modify method since it allows us to:

  • encapsulates mutation logic VS having to multiple statements
  • super flexible and can pretty much cover a whole range of different use cases like appendToArray, prepend etc, so I don't think having an appendToArray is too useful.

Another alternative:

Have the set method take an optional callback method which does the work of the proposed .mutate function. _This would require an addition to the Conf API?

//BEFORE
const array = store.get('array');
array.find(element => element.title === 'baz').src = 'something';
store.set('array', array);

//AFTER
store.set('some_arr', (array) => {
    array.find(element => element.title === 'baz').src = 'something';
    return array;
})

I agree with @asmironov _"convenience method that makes sense is modify (mutate)"

EDIT:

  • Ignore my previous comment above allowing set to accept a mutate function...I realized that if no value is present, then function wouldn't work out well.
@sindresorhus
Copy link
Owner Author

@sindresorhus sindresorhus commented Feb 12, 2019

This could also be useful:

Before

{
	label: 'Auto Hide Menu Bar',
	type: 'checkbox',
	checked: config.get('autoHideMenuBar'),
	click(item, focusedWindow) {
		config.set('autoHideMenuBar', item.checked);
		focusedWindow.setAutoHideMenuBar(item.checked);
		focusedWindow.setMenuBarVisibility(!item.checked);
	}
}

After

store.checkboxMenuItem(
	label: 'Auto Hide Menu Bar'
	click(item, focusedWindow) {
		focusedWindow.setAutoHideMenuBar(item.checked);
		focusedWindow.setMenuBarVisibility(!item.checked);
	}
)

I do this a lot in my apps and it would reduce the boilerplate.

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

Successfully merging a pull request may close this issue.

None yet
4 participants
You can’t perform that action at this time.