ifandelse / machina.js

js ex machina - finite state machines in JavaScript
http://machina-js.org/
Other
1.93k stars 147 forks source link

I'll submit a PR if you confirm this is a bug #170

Open sfrooster opened 2 years ago

sfrooster commented 2 years ago

I'm in the process of creating type definitions for this library (which, BTW, I still find to be the best FSM lib I've found among those available for 3 different languages - node/javascript, dotnet/C#, and golang) which I'll submit to DefinitelyTyped (as a further ambition I may submit a PR to update the docs, but don't hold your breath) and in looking at the function getHandlerArgs which is defined in both the FSM and the BehavioralFSM, I noticed what looks to be a typo or c/p error or.... in the FSM version.

For comparison, in the BehavioralFSM:

getHandlerArgs: function( args, isCatchAll ) {
    // index 0 is the client, index 1 is inputType
    // if we're in a catch-all handler, input type needs to be included in the args
    // inputType might be an object, so we need to just get the inputType string if so
    var _args = args.slice( 0 );
    var input = _args[ 1 ];                 // <=== here, input is defined and set
    if ( typeof input === "object" ) {          // <=== here, input is used
        _args.splice( 1, 1, input.inputType );
    }
    return isCatchAll ?
        _args :
        [ _args[ 0 ] ].concat( _args.slice( 2 ) );
}

and in the FSM:

getHandlerArgs: function( args, isCatchAll ) {
    // index 0 is the client, index 1 is inputType
    // if we're in a catch-all handler, input type needs to be included in the args
    // inputType might be an object, so we need to just get the inputType string if so
    var _args = args;
    var input = _args[ 1 ];                 // <=== here, input is (also) defined and set
    if ( typeof inputType === "object" ) {          // <=== but here, inputType is used
        _args.splice( 1, 1, input.inputType );
    }
    return isCatchAll ?
        _args.slice( 1 ) :
        _args.slice( 2 );
}

I'm currently working with a BehavioralFSM so it'd take some work to get an FSM going and test etc, but I've looked at enclosing scopes and I don't see inputType coming in as a closure or anything like that so I think it's really meant to be input vs inputType and just hasn't been noticed.

If you agree, I'm happy to submit a PR, I just didn't want to do so without confirming I'm not missing something.