Core: Restore Docs `useParameter` using `DOCS_PREPARED` #22118

tmeasday · 2023-04-17T04:00:17Z

Telescoping on #22120

What I did

Added a new event DOCS_PREPARED analogous to STORY_PREPARED that emits the parameters of the current docs entry.

That's the component's parameters for CSF docs (autodocs + .stories.mdx), and attached MDX docs.
That's the project's parameters for unattached MDX docs.

This is in contrast to #22071 which added more impactful events (PROJECT_PREPARED and COMPONENT_PREPARED) which we should think a little more about.

How to test

Run a sandbox, check toolbar (in particular backgrounds) behaviour
- Check attached MDX, unattached.
Check the same for composition!
See tests

Checklist

Make sure your changes are tested (stories and/or unit, integration, or end-to-end tests)
Make sure to add/update documentation regarding your changes
If you are deprecating/removing a feature, make sure to update
MIGRATION.MD

Maintainers

If this PR should be tested against many or all sandboxes,
make sure to add the ci:merged or ci:daily GH label to it.
Make sure this PR contains one of the labels below.

["cleanup", "BREAKING CHANGE", "feature request", "bug", "documentation", "maintenance", "dependencies", "other"]

Parameters are pulled from the component (for csf docs entries or the project for mdx docs entries).

quantizor · 2023-04-18T01:28:03Z

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?

tmeasday · 2023-04-18T03:21:08Z

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.

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 storyStoreV7. Were you using ssv7 (code splitting/on demand store) in 6.5 @probablyup?

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?

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.

quantizor · 2023-04-18T03:40:47Z

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 storyStoreV7. Were you using ssv7 (code splitting/on demand store) in 6.5 @probablyup?

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.

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.

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.

tmeasday · 2023-04-18T04:46:35Z

This is less likely to happen imo and addons can avoid this by listening to events...

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.

Otherwise you should probably discourage use of parameters to store addon settings, that’s what everyone uses them for.

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.

shilman · 2023-04-18T07:00:10Z

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!

shilman · 2023-04-18T07:06:20Z

@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!

…-simpler Restore `useParameter` using `DOCS_PREPARED`

quantizor · 2023-04-24T15:29:45Z

code/lib/manager-api/src/tests/stories.test.ts

+      Object.assign(fullAPI, api);
+
+      await init();
+      fullAPI.emit(STORY_PREPARED, {


Potential buggy test here @tmeasday, shouldn't this be DOCS_PREPARED?

cc @shilman

Thanks @probablyup

#22118 (comment)

storybookjs#22118 (comment)

tmeasday added bug addon: docs labels Apr 17, 2023

tmeasday requested review from shilman, ndelangen and JReinhold April 17, 2023 04:00

tmeasday added 7 commits April 17, 2023 14:40

Remove unnecessary code

6726d4a

Refactor prepareMeta to cache on store

01b4936

Added new DOCS_PREPARED event with parameters.

3767338

Parameters are pulled from the component (for csf docs entries or the project for mdx docs entries).

Store docs prepared parameters in the index

8c6b20a

Make getParameter work for both story and docs

38b13e8

Add an E2E test of parameters behaviour

a2801d1

Update exports

03fecfe

tmeasday force-pushed the tom/21798-fix-useParameter-simpler branch from 77b60e0 to 03fecfe Compare April 17, 2023 04:48

tmeasday changed the base branch from next to tom/21798-add-attached-mdx-to-index April 17, 2023 04:49

valentinpalkovic assigned shilman Apr 17, 2023

tmeasday mentioned this pull request Apr 18, 2023

[Bug]: useParameter() returns nothing for a docs page #21798

Closed

Base automatically changed from tom/21798-add-attached-mdx-to-index to next April 18, 2023 09:56

shilman approved these changes Apr 18, 2023

View reviewed changes

shilman changed the title ~~Restore useParameter using DOCS_PREPARED~~ Core: Restore Docs useParameter using DOCS_PREPARED Apr 18, 2023

shilman merged commit 2c89991 into next Apr 18, 2023
57 checks passed

shilman deleted the tom/21798-fix-useParameter-simpler branch April 18, 2023 10:03

shilman mentioned this pull request Apr 18, 2023

Revert "Core: Restore Docs useParameter using DOCS_PREPARED" #22140

Closed

tmeasday added the patch:yes Bugfix & documentation PR that need to be picked to main branch label Apr 19, 2023

tmeasday mentioned this pull request Apr 19, 2023

Core: Restore useParameter behaviour for Docs entries #22071

Closed

5 tasks

shilman added the patch:done Patch/release PRs already cherry-picked to main/release branch label Apr 24, 2023

shilman added a commit that referenced this pull request Apr 24, 2023

Merge pull request #22118 from storybookjs/tom/21798-fix-useParameter…

7448bb3

…-simpler Restore `useParameter` using `DOCS_PREPARED`

quantizor reviewed Apr 24, 2023

View reviewed changes

tmeasday added a commit that referenced this pull request Apr 26, 2023

Fix buggy test as per

7d9bb54

#22118 (comment)

tmeasday mentioned this pull request Apr 26, 2023

Core: Fix DOCS_RENDERED test #22255

Merged

5 tasks

JungHoe pushed a commit to JungHoe/storybook that referenced this pull request Apr 26, 2023

Fix buggy test as per

3c1256a

storybookjs#22118 (comment)

dannyhw mentioned this pull request Feb 5, 2024

React Native: Fix errors on compose story #25906

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Core: Restore Docs `useParameter` using `DOCS_PREPARED` #22118

Core: Restore Docs `useParameter` using `DOCS_PREPARED` #22118

tmeasday commented Apr 17, 2023 •

edited

quantizor commented Apr 18, 2023 •

edited

tmeasday commented Apr 18, 2023 •

edited

quantizor commented Apr 18, 2023 •

edited

tmeasday commented Apr 18, 2023 •

edited

shilman commented Apr 18, 2023

shilman commented Apr 18, 2023

quantizor Apr 24, 2023 •

edited

tmeasday Apr 26, 2023

Core: Restore Docs useParameter using DOCS_PREPARED #22118

Core: Restore Docs useParameter using DOCS_PREPARED #22118

Conversation

tmeasday commented Apr 17, 2023 • edited

What I did

How to test

Checklist

Maintainers

quantizor commented Apr 18, 2023 • edited

tmeasday commented Apr 18, 2023 • edited

quantizor commented Apr 18, 2023 • edited

tmeasday commented Apr 18, 2023 • edited

shilman commented Apr 18, 2023

shilman commented Apr 18, 2023

quantizor Apr 24, 2023 • edited

Choose a reason for hiding this comment

tmeasday Apr 26, 2023

Choose a reason for hiding this comment

Core: Restore Docs `useParameter` using `DOCS_PREPARED` #22118

Core: Restore Docs `useParameter` using `DOCS_PREPARED` #22118

tmeasday commented Apr 17, 2023 •

edited

quantizor commented Apr 18, 2023 •

edited

tmeasday commented Apr 18, 2023 •

edited

quantizor commented Apr 18, 2023 •

edited

tmeasday commented Apr 18, 2023 •

edited

quantizor Apr 24, 2023 •

edited