Vue3: Fix Conditional decorators

[chakAs3]

Closes #22945

What I did

How to test

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"]

[yannbf]

Could you please elaborate why this is needed?

[chakAs3]

i'll take it back it was a try

[yannbf]

@ndelangen @tmeasday would you mind taking a look? Thanks!

[kasperpeulen]

He Chakir,

Thanks for the PR. Can you make it a bit more focused on the problem? I have been trying to solve this as well. I'm sure it can be solved in a way that doesn't involve applying args twice, which we already discussed as something we don't want to do in SB7.

[chakAs3]

this disable attribute fallthough on all the prepared stories including decorated onces,

[tmeasday]

Doesn't this line mean the later decorators will get called even if the earlier decorator (currentDecoratorFn) never calls the passed storyFn (defined on line 81)?

As an aside it would help me at least (maybe it's obvious to others) to explain what's happening here -- why do we return storyResult sometimes, and other times we call prepare() on it?

[chakAs3]

exactly @tmeasday if you don't get a deccorated Story from the decorator or the decorator did you call storyFn, we just take the last one. so we can now have decorator that returns null, so we kind of skip them.
so now we can have

(storyFn: PartialStoryFn, context: StoryContext) => context.args.condition? storyFn(): null

[chakAs3]

we return storyResult sometimes, and other times we call prepare() on it?

we don't need to prepare it because the previous one is already prepared we just return it

[chakAs3]

we can have decorators that have just side effects and not touching the rendering Tree. by doing something like this.

(storyFn: PartialStoryFn, context: StoryContext) => {
       saveData(context.args.data)
      return null
  }

[chakAs3]

As an aside it would help me at least (maybe it's obvious to others)

😄 I think you are the only one or at least to first one who spotted the Key change in this PR that fix the conditional decorator issue

[tmeasday]

we can have decorators that have just side effects and not touching the rendering Tree. by doing something like this.

I'm confused. So a decorator that returns null would have "no effect" on rendering, in the sense that later decorators and the story function would still render? That's inconsistent with the other frameworks, and I think the opposite of what we are trying to achieve (consistency in decorator behaviour).

[chakAs3]

no of course the children won't render since the parent not providing any story, one thing is important I'm sure you know it is rendering should happen down -> up the tree.

decorators.mov

[chakAs3]

what I meant no effect on the parent so only if put it like last to run

decorators: [
    // conditional decorator, runs before last
    (storyFn: PartialStoryFn, context: StoryContext) => {
      return context.args.condition
        ? storyFn()
        : null;
    },
    // decorator A that uses hooks
    (storyFn: PartialStoryFn, context: StoryContext) => {
      useEffect(() => {});

      return storyFn({ args: { ...context.args, text: `WrapperA( ${context.args['text']} )` } });
    },
    // decorator B that uses hooks
    (storyFn: PartialStoryFn, context: StoryContext) => {
      useEffect(() => {});

      return storyFn({ args: { ...context.args, text: `WrapperB( ${context.args['text']} )` } });
    },
    
  ],

in this case, won't affect the rendered tree.

[chakAs3]

there is a repo on Stackblitz. if you want to test that fast without spawning any sandbox https://stackblitz.com/~/github.com/chakAs3/storybook-vue3-decorators

[kasperpeulen]

Let's keep this PR focused to the actual problem, fixing conditional decorators. Please revert all other changes.

You are really making changes to the core API here, as you are applying args twice, and args are now also available as props. We should not make such big changes to the public API, in a bug fix PR.

[yannbf]

@tmeasday @kasperpeulen what are the next steps in this PR?