Addon-docs/Angular: Fix inline story rendering

[tmeasday]

Issue: #16147

What I did

• Pass component properly into the inline rendering (it used to come off parameters)
• I also made component required, which it is (I believe), and meant that TS would have picked this up.

How to test

• Is this testable with Jest or Chromatic screenshots?
• Does this need a new example in the kitchen sink apps?

No, and I wonder if we can get a better test for this -- can we include an example with inline rendering somehow?

[nx-cloud]

Nx Cloud Report

CI ran the following commands for commit 868768f. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch

Status	Command
	nx run-many --target=prepare --all --parallel --max-parallel=15

---

Sent with 💌 from NxCloud.

[shilman]

@tmeasday i'll try adding an inline rendering test now

[ThibaudAV]

The decorateStory function is still called ? because it should have converted the component to a template

aaa or the component is not added in the metadata. Action done here

storybook/app/angular/src/client/preview/angular-beta/StorybookModule.ts

Line 75 in 5982475

...(requiresComponentDeclaration ? [component] : []),

This explains the absence of errors in the console

If my assumption is correct, we could have to move this code to decorateStory.

• getStorybookModuleMetadata would no longer be involved in declaring the component or the computesTemplateFromComponent
• decorateStory -> prepareMain converted the component to a template. And if this action is performed, the component is added to the metadata if it is not already declared

🤔 WDYT ?

otherwise it's good for me.

[tmeasday]

I'll admit I am not quite following @ThibaudAV. What are we doing differently in rendering Angular stories inline in 6.4 compared to 6.3? The intention was not to change it.

[ThibaudAV]

the component is no longer in StoryFnAngularReturnType.component

To summarise, Angular needs the component to be declared.
And there are 2 ways to do this either with the moduleMetadata or to let storybook do it.
And what has been broken is the second way only for docs

I think that the automatic declaration of the component by storybook comes too late in the process and should be done in decorateStory -> prepareMain. this would simplify the process. and there would be no need to carry the component around 💭
but this can be the subject of a dedicated PR :)

[ThibaudAV]

seems to have another problem I'm looking at a solution

[shilman]

@tmeasday I just walked through @ThibaudAV 's change with him. I'm fuzzy on the details, but the problem according to him is that prepareForInline is getting called twice per story, and Angular has problems with this. Presumably this was only called once during the previous version of Docs rendering. His change is a workaround that de-dupes the call, and also tries to mutex the concurrency between stories as well. If we can figure out the top-level prepareForInline issue, we might be able to revert the workaround.

[ThibaudAV]

Exactly,
maybe not revert all my change. we could keep the React.useEffect which seems to correct part of the problem

I also wonder if it is normal that when we change some controls in the docs that it does not refresh the component with their new value ?

[tmeasday]

@shilman let's look at this together today, we certainly shouldn't be doing that.

I also wonder if it is normal that when we change some controls in the docs that it does not refresh the component with their new value ?

This is new behaviour in 6.4? Do you mean something specific by "refresh"? Or do you just mean re-render? In any case no change in behaviour was intended.

[ThibaudAV]

@tmeasday

This is new behaviour in 6.4? Do you mean something specific by "refresh"? Or do you just mean re-render? In any case no change in behaviour was intended.

Yes re-render. or more tech prepareForInline is not called when some controls / args change
(for example if you change the button label )

I just tested with the tag v6.3.8 and it works

[shilman]

Merging this with a followup in #16223 for the CLI failure.