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

this._data is never set when without hooks path is taken in transition_impl #557

Closed
WNemencha opened this issue Dec 23, 2023 · 3 comments
Closed

Comments

@WNemencha
Copy link

WNemencha commented Dec 23, 2023

jssm/src/ts/jssm.ts

Lines 2362 to 2380 in 7de448c

// or without hooks
} else {
if (this._history_length) {
this._history.shove([ this._state, this._data ]);
}
this._state = newState;
// success fallthrough to posthooks; intentionally no return here
// look for "posthooks begin here"
}
// not valid
} else {
return false;
}

Code in this extract should change to something like:

+       this._data = newData || this._data; // or better logic not sure about this.

        if (this._history_length) {
          this._history.shove([ this._state, this._data ]);
        }

        this._state = newState;

otherwise when not using hooks, and calling .data() it always return null, also data does not appears in history.

@StoneCypher
Copy link
Owner

Sorry, took me a second to see this. In the future, I'll see it faster if you use the unified tracker.

I'm now trying to think my way through this problem.

@StoneCypher
Copy link
Owner

Confirmed. Resolution will continue on StoneCypher/fsl#1259

Thank you, @WNemencha

image

@StoneCypher StoneCypher reopened this Dec 28, 2023
StoneCypher added a commit that referenced this issue Dec 28, 2023
data arg2 wasn't implemted; only data argument in hooks.  implemented; added 26 tests to control.  fixes #557; fixes StoneCypher/fsl#1259
@StoneCypher
Copy link
Owner

data argument for transition, action, go, do via transition_impl wasn't actually wired up. it looked like it was, because the data return value from hooks was and they go through the same mutual implementor.

You were right that the call pattern had to be implemented without hooks, but I also had to implement the call pattern with hooks.

This led to noticing that at this time, there is also no way practical way to set data to undefined, which will be fixed promptly.

26 tests added to enforce:

  1. a common sense full cycle without hooks
  2. a common sense full cycle with hooks
  3. every adjacent pairing of hooks in ordering, to assert that hooks override in the correct order

Thank you, @WNemencha , for improving this state machine. I apologize for this oversight.

image

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

No branches or pull requests

2 participants