Core: Overhaul start.js and event emitting/listening

[tmeasday]

A refactor of the start.js / @storybook/client-api code base.

What I've done:

• Completed TS translation of @storybook/core/src/client/preview directory started by @kroeder in WIP: [ts-migration] lib/core #9870
• Pulled rendering code out of start.ts into StoryRenderer.tsx & wrote tests
• Pulled configure/loading code out of start.ts into makeConfigure.ts & wrote tests
• Rework the way we emit and listen to event across the preview to be more consistent and simpler
• Document the below in READMEs/etc

Previously things were a bit of a mess for a few reasons:

1. all the different "modules" in the preview (client api / story store / config_api / rendering / loading / url handling code) tended to have references to each other and call each other in fairly unpredictable ways.
2. event emitting was a mess. The store was an event emitter, however other modules "emitted" from the store (via 1). Plus things emitted on the channel. Other times things just called APIs on each other. There was no real system to it.
3. We debounced the emitting of stories over the channel to the manager when adding them to avoid sending them heaps of time. This had the side effect of introducing a timeout into emission which led to ugly workarounds and no one really knew when what happened.

Preview module system

This PR doesn't attempt to "fully" solve all of this and design a new module system from scratch (see notes below). Instead I just tried to rationalize things in the lowest impact way possible. Here's what I did:

A) Each module only has access to the channel and the store. They can calls APIs on the store if they wish.
B) Each module is responsible for listening to the events that drive their own behaviour from the manager or elsewhere in the preview (e.g. the rendering code listens to the RENDER_CURRENT_STORY event which is emitted by the store).
C) All modules should emit events (either "imperative" -- ie talking to another module, or "past tense" -- ie something just happened) on the channel.

Events on startup

To avoid a delay in emitting the SET_STORIES event, we simply wait until configuration is over before emitting it. This means the fact that you can't change the store outside of configuration is explicit. The configuration api calls startConfiguring() and finishConfiguring() on the store and the store will emit at the end.

Rendering current story

The StoryRenderer will render the current story from the store whenever RENDER_CURRENT_STORY is called. It will do the right thing whatever the state of the store is.

The store will emit that event when it thinks it is ready to renderer:

• Either an error was set (configure failed) OR
• It has a selection and configuration is over.

It will then call it whenever the selection changes (assuming configuration isn't in process).

Breaking changes

• There's a new type RenderContext which is passed to the render function of each framework. I pulled a few things out of it that I am pretty sure weren't being used and seemed like junk. This is definitely a BREAKING CHANGE. See

  storybook/lib/core/src/client/preview/types.ts

  Lines 40 to 59 in c4a8c86

  	// Previously this also included these fields but I don't think they were used:
  	// { configApi, storyStore, channel, clientApi, };
  	export interface RenderContext extends StoreItem {
  	id: StoryId;
  	kind: StoryKind;
  	name: StoryName;
  	parameters: Parameters;
  	getDecorated: () => StoryFn;
  	getOriginal: () => StoryFn;
  	hooks: HooksContext;
  	// Legacy identifiers that are already on StoreItem (as name/kind) but widely used
  	selectedKind: StoryKind;
  	selectedStory: StoryName;
  	forceRender: boolean;
  	showMain: () => void;
  	showError: (error: { title: string; description: string }) => void;
  	showException: (err: Error) => void;
  	}

• Note that selectedKind and selectedStory are no longer on the above.

Further improvements / bugs that aren't in this PR

• We could design a proper "module" system on the preview side. Perhaps we could mirror the way it works on the manager side for consistency, or if we don't like that, we could do something clean. I suspect client_api should be the interface that everything goes through (rather than things calling into the store).

• It's kind of weird the way that storyshots expects start.ts to not create a channel (based on some checks of the userAgent) and then later sets a mocked channel. Is there a cleaner solution here that isn't so tailored for SS?

• config_api could probably be pulled into client_api

• Can we drop setAddon from client_api: Remove setAddon code #10008

• Global params + decorators have some fundamental issues on HMR: Clearing global decorators / parameters does not work properly on HMR #10005

• I didn't implement async stories. We kind of want this in 6.0 (for @storybook/server) 🤷‍♂ : Support async stories #10009

[github-actions]

	Fails
🚫	PR is marked with "in progress", "BREAKING CHANGE" labels.

Generated by 🚫 dangerJS against b9af9f6

[jonspalmer]

Should we keep passing selectedKind and selectedStory in RenderContext? It already has kind and name from the story store. I realise these are used all over the place so this may be a bridge too far.

For what it's worth I found the duplication of selectedKind & selectedStory suer confusing as I was learning my way around the code base. If we could nuke them it would be a great win for clarity. Breaking changes in 6.0 or bust.....

[tmeasday]

OK, refactor is complete and at first blush appears to be working. I still want to add some tests (and I probably broke the existing ones).

Will update the top comment when I am done and call for review but @shilman @ndelangen -- do you have thoughts on the selectedKind/selectedStory thing?

[shilman]

I think it's fine to remove it if we provide migration instructions:

const { id, store } = context;
const { kind: selectedKind, name: selectedStory } = store.fromId(id);

[tmeasday]

@shilman it's not even that complicated, name and kind are already on the context :)

[tmeasday]

Whoops I hadn't even pushed what I was referring to above 😊

[shilman]

don't we need access to the store?

[tmeasday]

I added back the store just to docs context.

[shilman]

Great work! Lots of changes, but if there are no objections, I think we should merge and address issues in follow up PRs