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

Preserve default context on start from named state #573

Merged
merged 1 commit into from
Jul 26, 2019
Merged

Preserve default context on start from named state #573

merged 1 commit into from
Jul 26, 2019

Conversation

kousu
Copy link
Contributor

@kousu kousu commented Jul 26, 2019

When an initial state is not passed to service.start(), machine.initialState is used, which is filled in with a lot of details, particularly a copy of the default machine.context.

When an initial state is passed as a State object, it is loaded directly, and it is assumed that this comes with the desired .context value filled in.

But when an initial state is passed as a named state, like interpret(machine).start("COMMENT_FORM"), the resulting state has .context === undefined because nothing ever sets it. State.from(), which handles loading a string state to a full state object, already had an option to attach a context, but it was not used in this case.

Found during #432

When an initial state is not passed to `service.start()`, `machine.initialState` is used, which is filled in with a lot of details, particularly a copy of the default `machine.context`.

When an initial state is passed as a `State` object, it is loaded directly, and it is assume that this comes with the desired `.context` value filled in.

But when an initial state is passed as a named state, like `interpret(machine).start("COMMENT_FORM")`, the resulting state has `.context === undefined` because nothing ever sets it. `State.from()`, which handles loading a string state to a full state object, already had an option to attach a context, but it was not used in this case.
@davidkpiano davidkpiano merged commit 93ad34d into statelyai:master Jul 26, 2019
@Andarist
Copy link
Member

@kousu would u be willing to prepare a followup PR with a test covering this?

@kousu kousu deleted the named-state-contexts branch July 27, 2019 01:43
@kousu kousu restored the named-state-contexts branch July 27, 2019 01:43
@kousu
Copy link
Contributor Author

kousu commented Jul 27, 2019

@Andarist I'll take a look at the testing framework you've got going and get back to you.

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.

3 participants