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

Explain why it's not a singleton #15

Open
sindresorhus opened this issue May 22, 2017 · 2 comments
Open

Explain why it's not a singleton #15

sindresorhus opened this issue May 22, 2017 · 2 comments
Labels

Comments

@sindresorhus
Copy link
Owner

sindresorhus commented May 22, 2017

It's easy to think that it would make for a simpler API. No need to initialize it first. I thought so too at first.

But imagine if it had been a singleton:

index.js

const config = require('electron-config');
const someComponent = require('./some-component');

config.setDefaults({
  foo: true
});



config.get('foo');
//=> true

some-component.js

const config = require('electron-config');



config.get('foo');
//=> undefined

Note the undefined. So you would have to be careful about setting the defaults before any app imports, which is easy to forget.


The following a much safer and explicit pattern:

config.js

const config = require('electron-config');

module.exports = new Config({
  foo: true
});

index.js

const config = require('./config');
const someComponent = require('./some-component');



config.get('foo');
//=> true

some-component.js

const config = require('./config');



config.get('foo');
//=> true

Thoughts on this?

@feafarot
Copy link

I'd agree that it should not be a singleton. The ability to create multiple instances for multiple files is very helpful. Maybe it should be explained somewhere in the docs thou.

@thisismydesign
Copy link

@feafarot's explanation makes sense to me.

On another note, not sure about module.exports = new Config() but keep in mind that using ES6 modules export default new Config() is most likely not what you want. It will evaluate a single time during import and won't pick up changes to userData path, for example.

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

No branches or pull requests

3 participants