Thoughts on standardising state sources

[dariocravero]

Hi all,

I've been looking at state source testing and I think there might be some room for standardisation over there.

As it stands, a HttpStateSource -because of its async nature- returns promises and also dispatches actions once they get results. The rest just return stuff right away.

This means that its caller -generally Queries- should be aware of the state source's implementation thus coupling the two a little bit. The docs make it even more confusing by handling of that promise both, in Queries and in the HttpStateSource even though the API shows it using the ActionCreators.

So all in all, I see two issues:

• what a StateSource is supposed to return and
• where communicating its result to the application should happen.

I'd say every developer can make their own choices but we should explain the different paths in the docs.

Personally, I'd be inclined to suggest/teach/enforce a common return pattern in StateSources -probably promises because that's a restriction that the http ones- and also have its results handled by their callers. As in, they would just return that promise and let either the ActionCreators or Queries deal with it. As I said before, a developer could then consciously choose not to follow this pattern and that's totally fine, but at least they would understand that they wouldn't be getting interchangeability and independence between different types of sources then.

This also led me to think that we could explain or provide a pattern/docs on how to use multiple state sources chained. A practical use case for this would be: keep stuff in local storage but sync back to an http endpoint.

What are your thoughts on this? /cc @martyjs/core

[taion]

I thought most of the more recent examples show state sources returning futures, and the actions being dispatched from the query or action creator classes: http://martyjs.org/guides/fetching-state/index.html

Might be sufficient just to make the docs more consistent. Since the state sources are essentially just the equivalents of web API util libraries, having some flexibility there makes sense, but they are somewhat peripheral.

[dariocravero]

Yes, http examples do. I agree that flexibility makes sense and that's why I wouldn't enforce any of those to return promises when they aren't needed. However, since it's a Marty type, I would encourage that behaviour though for consistency.

[idolizesc]

Agreed. I extended the LocalStorage state source and it was largely pointless to interact with it inside the Action Creators (as I do with HTTP sources) because there were no promises to deal with -- instead I just ended up using the state source as the this.state part of my Store. Maybe not the most elegant, but adding Promises everywhere and doing it the other way seemed really unnecessary.

[taion]

:+1: to this - it's good to avoid Zalgo.

[CumpsD]

I just got bitten by this too, I followed the docs, used a LocalStorage source, but then found out it doesn't return any promises, so everything I wrote in Queries is wrong.

[taion]

The really perverse thing is that one of the main benefits of state sources is to give you global hooks to change behavior when running on the server so your code can stay universal/isomorphic, but LocalStorage admits no general server-side equivalent.

That said, is it really better to introduce async behavior when not needed? The move might just be to pull these out of the core library and into a separate one.

[CumpsD]

Pulling it out wouldn't be bad, no need for async when it is not needed. I just don't know how this would work then:

        return this.fetch({
            id: 'getLogging',
            locally: function () {
                return this.state.loggingEnabled;
            },
            remotely: function () {
                return this.app.loggingQueries.getLogging();
            }
        });

I guess you can't use the fetch if remotely doesnt return a promise?

[taion]

You actually can right now, though it will warn (and might hit some bugs). Fair point - will have to think about it.

[CumpsD]

I solved it like this now, not sure if it is according to the marty philosophy:

Store:

    get loggingEnabled() {
        return this.fetch({
            id: 'getLogging',
            locally: function () {
                return this.state.loggingEnabled;
            },
            remotely: function () {
                this.setState({
                    loggingEnabled: this.app.loggingQueries.getLogging()
                });

                return this.state.loggingEnabled;
            }
        });
    }

Queries:

class LoggingQueries extends Queries {
    getLogging() {
        var loggingStatus = this.app.loggingApi.getLogging();

        if (loggingStatus) {
            return loggingStatus;
        } else {
            return false;
        }
    }
}

Source:

class LoggingStorageApi extends LocalStorageStateSource {
    constructor(options) {
        super(options);

        this.namespace = 'exira';
    }

    getLogging() {
        return this.get('logging');
    }

[CumpsD]

Actually, I think it's bad what I did, since now if I ever change the Source to HTTP, I'll have to change my implementation of the store

[taion]

I can definitely see where the pain would be here - though I think even in your scenario you'd be better off dealing with it in the Queries object.

That said, I think it'd be a little weird to have an async local storage wrapper just to maintain API compatibility with the HTTP state sources. I guess maybe technically dispatches should be async anyway, but it's a weird problem.

I do think you should always go through actions on the remotely branch (though as observed you don't have to), but I don't have a good answer for this use case right now.

[CumpsD]

For now I'm using the API straight from the store for these, least pain. The entire use case is a deviation from flux anyway since it involves logging from all places (actions, stores, components, ...) and doing it over the dispatcher causes only more confusion. It's now just a logger class I attached to the Marty app to use from everywhere, with a store to keep some data combined.