Skip to content
This repository has been archived by the owner on Feb 25, 2024. It is now read-only.

Fixed an issue with machines created by withConfig/withContext not being captured #301

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

Conversation

Andarist
Copy link
Member

fixes #300

but... the console output after the fix is:

in-config log
withConfig log
in-config log

that is because we capture 2 machines here (and this is somewhat an expected result as per #214, just not really in this case). This shows the limits of the current eval-like approach. It's basically impossible to cover common scenarios correctly without static analysis, cc @davidkpiano

@changeset-bot
Copy link

changeset-bot bot commented Oct 19, 2021

⚠️ No Changeset found

Latest commit: d330137

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@vercel
Copy link

vercel bot commented Oct 19, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/statelyai/xstate-viz/Bf758rUEPCyTEf5KPZKn9exfctKF
✅ Preview: https://xstate-viz-git-andarist-fix-300-statelyai.vercel.app

@davidkpiano
Copy link
Member

This shows the limits of the current eval-like approach. It's basically impossible to cover common scenarios correctly without static analysis

The other solution here is to opt into reading from inspect(someMachine).start(), then interpreted machines can be visualized the same as how inspected machines are.

@Andarist
Copy link
Member Author

The other solution here is to opt into reading from inspect(someMachine).start(), then interpreted machines can be visualized the same as how inspected machines are.

Yes, we could differentiate the behavior based on the interpret calls being in the source file but that's kinda a hidden feature, people wouldn't easily realize the difference on their own.

Either way - this probably ain't a discussion for now. While this is gonna be slightly surprising - it's better than not supporting withConfig at all, right? 🤔 cc @mattpocock

@Andarist Andarist requested a review from mattpocock October 20, 2021 08:52
@davidkpiano
Copy link
Member

The other solution here is to opt into reading from inspect(someMachine).start(), then interpreted machines can be visualized the same as how inspected machines are.

Yes, we could differentiate the behavior based on the interpret calls being in the source file but that's kinda a hidden feature, people wouldn't easily realize the difference on their own.

Either way - this probably ain't a discussion for now. While this is gonna be slightly surprising - it's better than not supporting withConfig at all, right? 🤔 cc @mattpocock

Yep, this solution works for now

@mattpocock
Copy link
Contributor

@Andarist could we compare the config objects we receive and deduplicate them? That would prevent the doubling of logs.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Actions don't execute if specified via withConfig
3 participants