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

Emit "change" event (or the like) when new values are set. #27

Open
totherik opened this issue Feb 4, 2015 · 8 comments
Open

Emit "change" event (or the like) when new values are set. #27

totherik opened this issue Feb 4, 2015 · 8 comments

Comments

@totherik
Copy link
Contributor

totherik commented Feb 4, 2015

Thinking to possible future use-cases, it might be nice for consumers of this data to observe change events.

@totherik
Copy link
Contributor Author

totherik commented Feb 6, 2015

Have a couple of thoughts on this but would like to hear more ideas. I can think of 2 implementations of which we could use one or both:

config.on('change', function(key, newValue, oldValue) {
    // ...
});

config.subscribe('path:to:key', function (newValue, oldValue) {
    // ...
});

@totherik
Copy link
Contributor Author

totherik commented Feb 6, 2015

I suppose it could even be:

config.on('path:to:key', function (newValue, oldValue) {
    // ...
});

... but that feels off for some reason.

@aredridel
Copy link
Contributor

Filtering in a single event >> event naming problems

@aredridel
Copy link
Contributor

Change in config at runtime though is a whole can of worms!

@totherik
Copy link
Contributor Author

totherik commented Feb 6, 2015

Could you be more explicit?

@aredridel
Copy link
Contributor

Not all configurations make sense to change after the fact; some things may have to drop connections and reopen them to react to a configuration change; some may only initiate the configuration change for new actions, leaving the old uses in place, too, giving a 'transition period' effect.

There's lots of implications this opens up.

@pvenkatakrishnan
Copy link
Member

this has become more important now if we want to support configs from remote services on-the-fly

@jasisk
Copy link
Contributor

jasisk commented Mar 4, 2015

I started working on a solution for this but I'm not totally satisfied.

This is the solution I was working towards:

config.set('a:b:c:d', 'newValue');
// emits 'a:b:c:d' with 'oldValue' and 'newValue'
// emits 'change' with 'a:b:c:d'
// emits 'change' with 'a:b:c'
// emits 'change' with 'a:b'
// emits 'change' with 'a'
// emits 'change' with no arguments

This works with some down sides like:

  1. if newValue is an object, will always start the emitting chain even if they're equivalent in values.
  2. would rather get rid of change events, emitting on keys exclusively, but determining oldValue / newValue will be a disaster.

We can take care of those downsides if we adopt an immutable data structure for the store.

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

No branches or pull requests

4 participants