Original discussions:
In v1, it used to be possible to access the store in a synchronous manner via getState() or getValue(). These APIs were removed without a clear explanation.
We had this API in v1 of Store, and we removed it as people we basically abusing it. It's unlikely we'll add this API back in. The performance cost is near zero.
I see a lot of people arguing that I'm not "thinking reactive" enough if I try to access the store synchronously. I want to challenge that opinion.
The fact is that the current state is always available in memory. It's weird that I have to write async-style code to get a value that is then delivered synchronously from RAM.
I often need to get just the current state from the store. I know how to do that in a reactive way by using first() or take(1) before subscribing, I just don't want to write code like that. I don't see why I should have to create a subscription for a single value that is immediately available.
My previous comment on this subject received 5 upvotes so I know I'm not alone.
Our stance hasn't changed on including some sort of getState method. It encourages an anti-pattern of getting the state synchronously and its not something we want to promote in the library. If you need that access, you can extend the Store and add your own method.
What makes it an anti-pattern? I can see you're just trying to protect me from something but I don't know what.
@StevenLiekens it's an anti-pattern because it is an imperative break out of a functional pattern. There's almost always a more functional way of doing what is required.
You don't normally just need a value out of the Store - often, you need to perform some action in response to it. If you're using ngrx/rxjs, you should probably do that in a functional way.
An example could be using getValue() to get a value from the store on a click handler, to then dispatch an action. Rather than getValue and dispatch, this can be written using take(1).map().subscribe(store).
I do have click handlers like that, but they don't dispatch new actions. They perform browser actions like navigation or starting a download.
@StevenLiekens this may be less aesthetic but it is helpful IMO:
// assuming your inside an async function
const state = await store.pipe(first()).toPromise();
That's still async-style code for values that are always immediately available.
yep, it's a workaround that is sometimes useful. a solution will be either to override as @jinder suggested or change your code to be in a more functional style
If that's the recommended way to do it then can we at least have a nice API for it?
(I haven't used ngrx in a while so this may not make sense)
store.select(somethingSelector).getValue().subscribe(thevalue => ...);
instead of
store.select(somethingSelector).pipe(take(1)).subscribe(thevalue => ...);
@StevenLiekens it's not the recommended way- it's a workaround for actually writing functional style code.
in any case .pipe(first()) seems reasonable to me (and part of the standard rxjs)
Then what is the recommended way for getting a snapshot of the state?
The recommended way according to this lib authors is not getting a snapshot but instead react to store changes and actions (i.e., functional programming). (BTW, I'm not one of those authors just repeating what @brandonroberts said)
Okay then back to my original statement: sometimes I don't care about changes and actions. I just want the current state and I want it now. Synchronously, not in a promise or subscription callback.
I sort of understand how publishing an API for it would promote wrong usage of the library. Obviously you'll get people who will write code that polls getState() on an interval instead of subscribing to changes. But why would you care about that?
This lib authors want to promote a different programming style - you can however extend the store or even fork the source.
Here's an example of working with a legacy API where being forced to write reactive code actually makes things worse.
The legacy API expects you to provide a callback and return a value from that callback.
It's important to note that the workaround isn't even possible because you're expected to immediately return a value from the callback.
private currentState: any;
private subscription: Subscription;
constructor(private store: Store<State>, private legacyService: LegacyService) { }
ngOnInit() {
this.subscription = this.store.select(something).subscribe(values) => {
this.currentState = values;
});
legacyService.oncallback = args => {
// do something with args + current state
var result = doSomething(args, this.currentState);
return result;
}
}
ngOnDestroy() {
this.subscription.unsubscribe();
}
If there was a synchronous API then you can cut out the subscription and the local state.
constructor(private store: Store<State>, private legacyService: LegacyService) { }
ngOnInit() {
legacyService.oncallback = args => {
const currentState = this.store.select(something).getValue();
// do something with args + current state
var result = doSomething(args, currentState);
return result;
}
}
You can argue that my imaginary legacy API design is shit to begin with but what if this legacy API is a browser API or something else that can't be easily changed?
@StevenLiekens take(1).subscribe() will run immediately if there's already a value. So you can set the variable locally, and return it outside of the subscribe method.
Look at me with a straight face and tell me you think this is better.
ngOnInit() {
legacyService.oncallback = args => {
let currentState: any = undefined;
this.store.select(something).pipe(take(1)).subscribe(values => currentState = values);
var result = doSomething(args, currentState);
return result;
}
}
@StevenLiekens It's not better, but your requirement is an edge case. In any case, it's trivial to wrap all that in a helper method that takes a Store parameter and will hide the implementation details for you.
I can think of other examples where a synchronous API is not a hard requirement but definitely an improvement.
@StevenLiekens And I'm sure you can build a helper method for that too :)
Not without losing sleep over it. Maybe if someone gave me a reason why this shouldn't be in the library that isn't "because we don't want to".
It might be a big nono, but right now we don't want to use the redux patter throughout our application, just in same cases, e.g. authentication and usermanagement. (We could implement it for the whole app, but right now it seems to much effort compared to the gains.) But I still need for example the users first name to show in a popup message. I can use the .take(1).subscribe() method, but in this case a snapshot seems more straightforward.
It's a bit like angular's ActivatedRoute and it's snapshot. You could get everything from the ActivatedRoute's observables and people can misuse/abuse the snapshot, but in some cases it's way simpler to use it.
I sort of understand why the team members don't like the idea of their product is used in the wrong way (especially when people starts to complain after that), but I'm still feeling like a child who is being protected from himself. Maybe that's the part that bothers me the most? :)
Hey @brandonroberts I just wanted to say the competition does have this feature.
https://ngxs.gitbook.io/ngxs/concepts/store#snapshots
Is now a good time to reconsider yet?
/edit
NGXS also has a Store.selectOnce() which is the same as select().pipe(take(1)) but much more expressive.
@StevenLiekens @fpeterdev I've gotten by using an extension like this:
https://gist.github.com/arenglish/91cfc234cc584e2006c561c9bbca6929
It basically subscribes to the selector, takes the first emission, then returns it like getValue() (which I never had the chance to use, so I'm not sure how it handled this).
I was disappointed to find out that ngrx didn't have a snapshot select, but I understand how it goes against the philosophy.
@jinder https://github.com/ngrx/platform/issues/227#issuecomment-408899204
I've found that method of returning a variable outside the store.select subscription which was set inside the subscription extremely confusing as you typically don't expect callbacks to happen synchronously. I use the extension simply for readability so that it was less confusing reading through code trying to keep track of what was synchronous and what was asynchronous.
It's a similar benefit as what we get from async/await; code readability in places where non-blocking synchronous execution is okay. If a store select is synchronous then why not make it look synchronous?
@arenglish I'm not the library author, so it's not really me you need to convince :) . However, I imagine what they're trying to tackle is the ease at which in many cases users will jump to use this when it's not necessary and is in fact an anti-pattern. Especially when they're new to the framework.
Sometimes it is necessary to just select one state object. Generating a report is a simple example where you just want the current values and don't care about state changes after the report is generated. I say making that code look like asynchronous code is the real antipattern.
I'm unsubscribing from this thread as we're going round in circles. In any case, feel free to reach out to the library authors to see if they want to change it.
Fair enough. I just don't see why people are so passionate about keeping this feature out of the library. I think there is enough demand for it, considering how many people do end up writing their own extension.
Protecting new users is a weird reason not to offer advanced features. You're not my nanny.
We won't do this. Subscribing to Store will always be guaranteed to be synchronous if you absolutely need to interact with state in an imperative way.
Most helpful comment
What makes it an anti-pattern? I can see you're just trying to protect me from something but I don't know what.