reflux / refluxjs

A simple library for uni-directional dataflow application architecture with React extensions inspired by Flux
BSD 3-Clause "New" or "Revised" License
5.36k stars 330 forks source link

Documentation not clear on getting promise from actions #524

Closed twgraham closed 7 years ago

twgraham commented 7 years ago

I know theres been a few issues in the past around async actions and promises, namely #216 and #223. However, I feel as though the documentation for Reflux still lacks in this area, and as a user on the newer versions of reflux (3+), I cant work out how I'm supposed to get promises from actions. I have tried using the reflux-promise library with refluxjs 6+, however when I try:

const ProjectActions = Reflux.createActions([{
    'createProject': { asyncResult: true }
}])

ProjectActions.createProject().then(...).catch(...)

I receive an error telling me .then() is undefined

For example say I have a new object form (component) that calls an action - in turn a store - that POSTS to a web API when the user submits. I need to know the result of that action in order to know how I should respond i.e. should I take them to the page that shows them the new object in detail, or should I stay and show an error. I don't think it makes sense to put this routing logic inside any of the stores, because this routing is specific to this component - i.e. I could create an object (or many) from another component and not want to route.

BryanGrezeszak commented 7 years ago

As far as Reflux itself lacking documentation on it, that's by design. It is all split out into reflux-promise, and the promise based docs are all in there, not in Reflux itself.

As for the actions not returning the promise, I think that's a bug introduced when a few action changes were made a bit ago. I'll look into it. But in the mean time you can likely just get around it by calling myAction.triggerAsync('foo') directly instead of just myAction('foo')

However, I'm also confused by some stuff you're saying. You're talking about defining how an action would work...but you're giving examples in the context of calling that action...not of defining behaviors of that action.

Defining what an async action does goes something like this:

// defining the action itself
const ProjectActions = Reflux.createActions({
    'createProject': { asyncResult: true }
})

// now defining what actually happens in that action (Reflux alone)
ProjectActions.createProject.listen( function(myArg) {
    somethingPromisy(myArg)
        .then(ProjectActions.createProject.completed);
        .catch(ProjectActions.createProject.failed);
});

// OR, if you use reflux-promise, it can reduce that to a simpler 1-liner
ProjectActions.createProject.listenAndPromise( somethingPromisy );
twgraham commented 7 years ago

Hey @BryanGrezeszak thanks for clarifying. Still it might be useful to atleast mention the promise repo in the docs - i was only able to find it was needed through past issues and the changelog.

Sorry ill try give a better example/more relevant code when im able to. However im not trying to define what the action does in the component. I have a ProjectStore which has the createProject() behaviour defined, i.e. it makes rest calls to create a project and takes a successful response adding it to the store state.

All i want to be able to do is call the action and determine if the action was successful or not for the specific calling component.

Ill try your first suggestion (triggerAsync). Otherwise are you suggesting that i should have a react component listen to the completed and failed child actions of the action i call?

twgraham commented 7 years ago

Just checked, I can't get promises using triggerAsync() either.

My bootstrap contains:

import Reflux from 'reflux'
import RefluxPromise from 'reflux-promise'

Reflux.use(RefluxPromise(window.Promise))

In my component I call:

ProjectActions.createProject.triggerAsync(newProject)
  .then(logger.info("hooray"))
  .catch(logger.error("aww man..."))

And i get the error: Cannot read property 'then' of undefined

More context (actions, store, component):

export const ProjectActions = Reflux.createActions([{
    'listProjects': { asyncResult: true },
    'createProject': { asyncResult: true }
}])

export default class ProjectsStore extends Reflux.Store {
  constructor() {
    super()
    this.listenables = ProjectActions
    this.state = {
      projects: []
    }
  }
  ...
  createProject(project) {
        rest('POST', '/projects', project)
            .then(createdProject => {
                const { projects } = this.state
                const updatedProjects = update(projects, { $push: [createdProject] })
                this.setState({ projects: updatedProjects })
        })
  }
}

export default class NewProjectForm extends React.Component {
  ...

  _create() {
        const newProject = this.state.project
        ProjectActions.createProject.triggerAsync(newProject)
          .then(() => {
            logger.info("hooray")
            // Now i know it was successful, I want to route to the new project page... say '/projects/{id}
            // And i only want to route to the new page from this flow - hence this logic isn't in the store
           })
          .catch("aww man...")
  }
}
BryanGrezeszak commented 7 years ago

You missed the most important part of my response: about how I don't actually see any proper defining of what the asyncResult action does in your code.

The reason you're not getting a promise back from your asyncResult action is not because there's something messed up with the promises.

It's because you don't have a proper asyncResult action set up anywhere.

I don't see anywhere where you're actually dealing with the completed and failed child actions of it, or using the reflux-promise shortcuts for doing so.

This isn't something specific to newer versions of Reflux, or to reflux-promise. It's fundamental to dealing with asyncResult actions in Reflux. If you don't have a proper action set up, then all the other problems down the line are just stemming from that.

This is how an asyncResult action works in Reflux:

// asyncResult means it has `completed` and `failed` child actions automatically
var Actions = Reflux.createActions({
    'load': {asyncResult: true}
});

// now we tell it what to actually do with those child actions
Actions.load.listen((file)=>{
    request(file)
        .then( result => Actions.load.completed(result) )
        .catch( result => Actions.load.faileded(result) )
})

// ----------------------- probably in another file ... ----------------------

// now any store just listens for the RESULT of the action, the completed or failed
class ExampleStore extends Reflux.Store
{
    constructor() {
        super();
        this.state = {text: ''};
        this.listenables = Actions;
    }

    // listen for Actions.load.completed, since we did listenables
    // that basically means we can just make `onLoadCompleted`
    onLoadCompleted(result) {
        this.setState({text: result});
    }
}

Now any time Actions.load("some-file.txt") is called, the action handles it itself, and stores are updated as to the result of the completed action.

The stuff in reflux-promise gives you shortcuts for doing some of that and such...but it still follows the principal.

twgraham commented 7 years ago

Okay I understand, thanks again, I really do appreciate the help. I apologise if my question seems simple, I just haven't been able to get this to work with whats documented in front of me - there's generally not a lot of examples with asynchronous actions. I've primarily used another repo, Graylog, to learn but even then they're using 0.2.x which has quite a few differences. The result I'm trying to achieve is put well as the last example here

So stripping it back:

I still don't get a promise back when I call MyActions.doSomething(), or MyActions.doSomething.triggerAsync(), like the example shows in the link above.

corneldm commented 7 years ago

I just revived an old branch — previously working on an old version of Reflux — and ran into this issue with promises. Looking at the source, it looks like reflux-promise is still needed to get a promise back from a call to an async action. If that's true though, we should update the documentation and address reflux-promise's dependency on reflux-core ^0.3.0 (incompatible with 1.x.x).

corneldm commented 7 years ago

reflux-promise does the trick, though I had to explicitly call triggerAsync

@twgraham now that your async actions are properly set up, try putting reflux-promise back in. Everything work as expected?

BryanGrezeszak commented 7 years ago

@twgraham - I did not say that reflux-promise is not needed if you want to use promises. I said that you had a fundamental problem in your Reflux usage that breaks things long before reflux-promise comes into the picture at all anyway, and that's why your reflux-promise usage is broken as well.

reflux-promise builds on top of normal Reflux usage. That's why you needed to get the normal Reflux usage down before reflux-promise will work.

twgraham commented 7 years ago

@corneldm yeah I saw that in the dependencies too - hence my hesitation to initially include reflux-promise because it looked like it wasn't being used for newer versions of reflux. What version of reflux were you using when you say reflux-promise did the trick (with triggerAsync). I've tried re-adding reflux promise like in some of my examples above, with the actions correctly setup, and I still have the issue of no promises being returned by the original action call. As you say maybe its the incompatibility of ^0.3.0 with 1.x.x

@BryanGrezeszak sorry I made that assumption since none of your examples really used reflux-promise (except where you said it could be used alternatively as a convenience 1-liner) - I still didn't see anywhere you actually used a promise from the original action call. I understand child actions, thats not what I'm really asking about though.

BryanGrezeszak commented 7 years ago

@twgraham - Once you do those things your original usage of reflux-promise works (well, when you use .triggerAsync anyway, like mentioned).

Nothing about your reflux-promise usage was messed up, only the Reflux asyncResult type action usage itself that reflux-promise then relies on.

So once you've got it set up to use completed and failed properly then Actions.load.triggerAsync("some-file.txt").then(...) (in the context of the example I gave above) starts working.

So that's why I'm not giving reflux-promise examples directly (well, not after my first reply anyway) because doing so just seemed to confuse the issue, because your problem wasn't actually in your reflux-promise usage.

My exact example above, once you add reflux-promise in (i.e. include the repo and run Reflux.use(RefluxPromise(Promise));), then returns a promise when you call the action with triggerAsync.

appelgriebsch commented 7 years ago

I’ve just had the same problem and found a solution more by incident (aka trail n error).

If you work with a module system (import, require, …) usually the modules get initialised with their own private version of the Reflux object (aka they don't use the global window object). As of this you might have to duplicate the Reflux.use(RefluxPromise(Promise)) call to all your action modules (see https://github.com/appelgriebsch/electron-shell-services/blob/master/actions/TranslationManager.jsx#L3-L5). Additionally any action call has to be triggered with triggerAsync to get the promise object in return….(see https://github.com/appelgriebsch/electron-shell/blob/master/app/shell/Shell.jsx#L80-L107).

That works for me.

vinnymac commented 7 years ago

I was working on a codebase that was written entirely in CoffeeScript and I was in the midst of translating it to babel. After bulk translating it everything worked fine except for redux async actions missing a then. I found this issue and I have to say thanks to @appelgriebsch you saved me tons of time. I ended up making a file that exports redux after adding promise support and just requiring it wherever I need to use reflux. I hope the below snippet helps others. It is a better option than duplicating the code everywhere.

import Reflux from 'reflux'
import refluxPromise from 'reflux-promise'
Reflux.use(refluxPromise(window.Promise))

export default Reflux

The project was running on these versions of reflux.

{
    "reflux": "0.4.1",
    "reflux-core": "0.3.0",
    "reflux-promise": "1.0.4"
}