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

WIP: Add promises returns for deferred calls #160

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

RickyCook
Copy link

@RickyCook RickyCook commented Oct 24, 2018

  • Return promises from deferUntilTransition
  • Resolve promises with return value from this.handle on processQueue

This allows values to be passed out of the FSM before a state transition happens:

fsm = machina.Fsm({
  ...,
  states: {
    uninitialized: {
      '*': function() {
        this.deferUntilTransitation();
        this.transition('start')
      }
    },
    start: {
      getValue: function() {
        return 'the value'
      }
    }
  }
})

assert await fsm.handle('getValue') === 'the value'

This change is Reviewable

@@ -1067,6 +1067,30 @@ function runBehavioralFsmSpec( description, fsmFactory ) {
}
] );
} );
( Promise ? it : it.skip )( "should return a promise when an action is deferred", function( done ) {
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unsure how you'd like this handled. I figured that for the moment I'd be safer, and just skip the test if Promise isn't available but this might not be the best option.

args: clientMeta.currentActionArgs
};
if ( callback ) {
queued.callback = callback;
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This breaks existing tests for the defer* functionality because there's an extra property there. Is there any way in the test suite that we can ignore this property? I couldn't find anything in the should.js documentation, but I've never used that lib before so I may have missed something

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

Successfully merging this pull request may close these issues.

None yet

1 participant