-
-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Fix state.can()
from restored state
#3097
Conversation
🦋 Changeset detectedLatest commit: 1b2f3bd The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
CodeSee Review Map:Review in an interactive map View more CodeSee Maps Legend |
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 1b2f3bd:
|
@Andarist is this good to go? |
packages/core/test/state.test.ts
Outdated
@@ -905,6 +905,27 @@ describe('State', () => { | |||
|
|||
expect(nextState.can({ type: 'NEXT' })).toBeTruthy(); | |||
}); | |||
|
|||
// https://github.com/statelyai/xstate/issues/3096 | |||
it('should work with restored state', () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We have a dedicated rehydration
test suite so perhaps it would be worth moving this there. I didn't notice at first that you already have a fix for this and I put such a test in that file locally:
it('should get correct information back from `can` immediately', () => {
const machine = createMachine({
on: {
FOO: {
actions: () => {}
}
}
});
const persistedState = JSON.stringify(interpret(machine).start().state);
const restoredState = JSON.parse(persistedState);
const service = interpret(machine).start(restoredState);
expect(service.state.can('FOO')).toBe(true);
});
Both are pretty much equivalent, so I don't mind using any of those.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only this one here depicts better what actually got reported - the fact that under the hood we call machine.resolveState
could be considered an implementation detail.
packages/core/test/state.test.ts
Outdated
const state = State.create( | ||
JSON.parse(JSON.stringify(machine.initialState)) | ||
); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this feels rather irrelevant for this test case - if we want to test that "machineless" state returns false
then that should be moved to a dedicated test
Co-authored-by: Mateusz Burzyński <mateuszburzynski@gmail.com>
Fixes #3096