-
-
Notifications
You must be signed in to change notification settings - Fork 9.4k
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
Core: Restore Docs useParameter
using DOCS_PREPARED
#22118
Conversation
Parameters are pulled from the component (for csf docs entries or the project for mdx docs entries).
77b60e0
to
03fecfe
Compare
Copied from the other thread: I’d really like to avoid returning nothing during render to avoid FOUC inside the preview window. It would make the dark mode addon have imperfect UX since it couldn’t proactively set dark mode based on config in preview.js. Really would just love parameters exported from preview to be the default, it would solve all problems I’m aware of. And why wouldn’t you? Isn’t anything in preview.js a global by default? |
My understanding (based on playing with it and reading the code), is that is what the addon does already in 6.5, at least if you are using
Yes but it can be overridden on a per-component or per story basis. So you could be browsing from one story to another that both disable an addon, and we could briefly fallback to the project parameters which enable it, and weird things might happen. |
Have not been because CSF doesn’t support dynamic stories, have a big dark mode and variant story generation system I’d love to convert over to a CSF-equivalent when that someday becomes available.
This is less likely to happen imo and addons can avoid this by listening to events… keeping the default parameter store populated is more important to ensure a consistent experience. Otherwise you should probably discourage use of parameters to store addon settings, that’s what everyone uses them for. |
OK, sure but making a change in a patch release (and maybe breaking such addons) is the issue here. I'm open to discussion on what behaviour we want in 7.1 and beyond.
To be clear most addons use parameters to configure their behaviour in the preview. I'm not sure using parameters to configure manager-side behaviour is a great idea because of exactly this timing issue. I'm not totally sure what the best alternative is though. |
There's a common pattern where addons (backgrounds, i18n, etc) store their state in SB globals, and then parameters set on the story level are used to update the value of the globals are updated by a decorator also provided by the addon. This is admittedly ugly, but there's no other declarative way to update globals. Would love to find a good way to handle this in the future and it could put a whole swath of related issues--including this one--to bed. Easier said than done, of course! |
@probablyup Re dynamic stories in CSF, we'll be proposing something this quarter over at #9828 in the next month or so. Would love to get your feedback once it's ready. Not a small change, as I'm sure you're aware! |
useParameter
using DOCS_PREPARED
useParameter
using DOCS_PREPARED
…-simpler Restore `useParameter` using `DOCS_PREPARED`
Object.assign(fullAPI, api); | ||
|
||
await init(); | ||
fullAPI.emit(STORY_PREPARED, { |
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.
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.
Thanks @probablyup
Closes #21798
Telescoping on #22120
What I did
Added a new event
DOCS_PREPARED
analogous toSTORY_PREPARED
that emits the parameters of the current docs entry..stories.mdx
), and attached MDX docs.This is in contrast to #22071 which added more impactful events (
PROJECT_PREPARED
andCOMPONENT_PREPARED
) which we should think a little more about.How to test
Checklist
MIGRATION.MD
Maintainers
make sure to add the
ci:merged
orci:daily
GH label to it.["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]