-
-
Notifications
You must be signed in to change notification settings - Fork 9.3k
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
Manager API: Fix api.getAddonState
default value
#23804
Manager API: Fix api.getAddonState
default value
#23804
Conversation
The confusion comes from a race condition: When using So it's recommended to only have the default state value defined in 1 place. everywhere else you might get an |
Oh I see.. so it's more involved than I originally thought. But my fix merely uses the default state provided to |
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.
Comitted on this one, so I can not approve anymore 😅
defaultState
when api.getAddonState
returns undefined
.defaultState
when api.getAddonState
returns undefined
.
defaultState
when api.getAddonState
returns undefined
.api.getAddonState
default value
Closes #23741 #23787
What I did
I've noticed
api.getAddonState()
sometimes (not sure the exact condition) returnsundefined
, which affects consumers ofapi.setAddonState()
.More specifically, #23787 occurs when the state updater function
setCount()
incode/addon/actions/src/manager.tsx
attempts to accessc.count
assuming the state{ count: number; }
is available all the time (i.e.,setCount((c) => ({ ...c, count: c.count + 1 }));
). Unfortunately as I mentioned above, sometimesapi.getAddonState()
, which is the value of the parameterc
of the callback insetCount()
, results inundefined
.This PR fixes the issue by providing
defaultState
whenc
is undefined inuseSharedState()
.How to test
Checklist
MIGRATION.MD
Maintainers
ci:normal
,ci:merged
orci:daily
GH label to it to run a specific set of sandboxes. The particular set of sandboxes can be found incode/lib/cli/src/sandbox-templates.ts
["cleanup", "BREAKING CHANGE", "feature request", "bug", "build", "documentation", "maintenance", "dependencies", "other"]
🦋 Canary release
This PR does not have a canary release associated. You can request a canary release of this pull request by mentioning the
@storybookjs/core
team here.core team members can create a canary release here or locally with
gh workflow run --repo storybookjs/storybook canary-release-pr.yml --field pr=<PR_NUMBER>