[Bug]: storybook 7 ignores Angular default values

[nshimiye]

Describe the bug

Consider sb@6 story below

export default {
  title: 'Example/Button',
  component: Button,
} as Meta<Button>;

export const Spacing =  (props: Button) =>  ({
  props,
  template: `
    <p>Minimum recommend gap is 1rem</p>
    <div style="display: flex; gap: 1rem;">
      <storybook-button [label]="label" [size]="size" [primary]="primary" (onClick)="onClick($event)"></storybook-button>
      <storybook-button [label]="label" [size]="size" [primary]="primary" (onClick)="onClick($event)"></storybook-button>
    </div>
  `
});

The storybook ui displays buttons with default label and controls with default values applied (label=Button and size=medium)

sb6 screenshot

If this story is translated into sb@7 equivalent,

const meta: Meta<Button> = {
  title: 'Example/Button',
  component: Button,
};
export default meta;

export const Spacing: StoryObj<Button> = {
  render: (props) => ({
      props,
      template: `
        <p>Minimum recommend gap is 1rem</p>
        <div style="display: flex; gap: 1rem;">
          <storybook-button [label]="label" [size]="size" [primary]="primary" (onClick)="onClick($event)"></storybook-button>
          <storybook-button [label]="label" [size]="size" [primary]="primary" (onClick)="onClick($event)"></storybook-button>
        </div>
      `
    })
};

The ui displays buttons with no default values. The control panel also indicates that these default values are not being used (i.e. label=undefined and size=undefined)

sb7 screenshot

Is this difference expected?

• If yes, would you recommend the right way to refactor the story
• If no, is there a plan to port sb6 behavior to sb7?

To Reproduce

• Storybook 6
• Storybook 7

System

# Storybook 7
Environment Info:

  System:
    OS: Linux 5.0 undefined
    CPU: (8) x64 Intel(R) Core(TM) i9-9880H CPU @ 2.30GHz
  Binaries:
    Node: 16.14.2 - /usr/local/bin/node
    Yarn: 1.22.19 - /usr/local/bin/yarn
    npm: 7.17.0 - /usr/local/bin/npm
  npmPackages:
    @storybook/addon-essentials: ^7.1.0-alpha.7 => 7.1.0-alpha.7 
    @storybook/addon-interactions: ^7.1.0-alpha.7 => 7.1.0-alpha.7 
    @storybook/addon-links: ^7.1.0-alpha.7 => 7.1.0-alpha.7 
    @storybook/angular: ^7.1.0-alpha.7 => 7.1.0-alpha.7 
    @storybook/blocks: ^7.1.0-alpha.7 => 7.1.0-alpha.7 
    @storybook/testing-library: ^0.0.14-next.2 => 0.0.14-next.2

Additional context

I was able to get around the issue by using an enhancer.

However, not sure if this is reliable, since I had to dig into Storybook codebase to figure out how to use it.
Moreover, it feels like if Storybook team wanted consumers to use argsEnhancers api, they would document it and probably provide sample enhancers

// .storybook/preview.js
const preview: Preview = {
  argsEnhancers: [
    ({ argTypes }) => {
      const label = argTypes.label.table.defaultValue.summary;
      const size = argTypes.size.table.defaultValue.summary;
      return { label, size };
    }
  ],
  parameters
};

[ShaunEvening]

Duplicate of #21300

[tmeasday]

Thanks for the reproduction @nshimiye.

My TLDR here is I don't know how this is happening. The object that's passed into Angular's renderer doesn't have any of those props set:

storybook/code/frameworks/angular/src/client/render.ts

Lines 21 to 26 in 03b85f4

	await renderer.render({
	storyFnAngular: storyFn(),
	component,
	forced: !forceRemount,
	targetDOMNode: element,
	});

I just don't know enough about Angular or SB's angular renderer to explain how it doesn't end up falling through to the default props. Is it because your template looks like this?:

 <storybook-button [label]="label" [size]="size" [primary]="primary" (onClick)="onClick($event)"></storybook-button>

If label etc are not passed into a such a template, what does Angular do? Does it fall back to the storybook-button's defaults? If not, I guess you need to call the button differently in that template.

[miszol1]

I just want to mention that the same template and inputs with undefined value in SB 6 was working properly and default values were taken from component even if they were not set in story props

[tmeasday]

@miszol1 yes, because in SB6 we were doing our "own" default value setting (ie. args would start with the values that we inferred as the defaults). This was bad for a few reasons:

1. The inference was not completely reliable and there were many bugs related to it getting defaults wrong.
2. We want to move to a world where inference is an optional extra and not required to render stories, for performance reasons (inference is slow).
3. It just doesn't make sense to never let the component fall back to the default, in a sense you are not testing it properly anyway by always setting the prop to the default value rather than leaving it undefined.

[miszol1]

So if I understood well, in SB7 with angular I won't be able to use default values in stories?

[tmeasday]

Hmm, well I'm not 100% sure about it but I think it's just that you need to be careful not to use templates that assume the args get set to the default values.

[miszol1]

This solution does not make any sense as story needs controls to be changed by user either components need default values. In my case i pass formGroups through inputs to components because it is not possible to do it without template - formGroup in angular is a circular structure and can't be passed through storybook's control.

[tmeasday]

@miszol1 sorry I am not quite understanding what you mean there. Could you provide a simple example of what you are trying to do? There may be another way to do it.

[miszol1]

Sorry that I explained it imprecisely. I mean that in my case I can't use non-template stories because of other blockers. Does it mean I am not able to use default values in SB7, right?

[tmeasday]

@miszol1 I'm not sure. I don't know a whole lot about angular templates but there may be a way to write them that doesn't rely on the default value being passed. Something like:

  // if label is not `undefined`
          <storybook-button [label]="label"></storybook-button>
  // otherwise
          <storybook-button></storybook-button>

Simply writing the first half is assuming (given the way that templates + defaults work) that label will always be set, which is not true (any more).

[marek-aguita]

@miszol1 sorry I am not quite understanding what you mean there. Could you provide a simple example of what you are trying to do? There may be another way to do it.

here I'm having a non-template story and the defaults do not work anyway:

@Component({
  selector: 'sg-inline-badge',
  templateUrl: './inline-badge.component.html'
})
export class InlineBadgeComponent {

  @Input() text: string;
  @Input() color: 'accent' | 'warn' | 'success'= 'accent';
}

export default {
  title: 'Components/Inline Badge',
  decorators: [
    moduleMetadata({ imports: [InlineBadgeModule] })
  ],
  component: InlineBadgeComponent,
} as Meta<InlineBadgeComponent>;

export const InlineBadge: StoryObj<InlineBadgeComponent> = {
  args: {
    text: 'Some text'
  },
  render: args => ({ props: args })
};

@tmeasday If I understand your point correctly, you think the problem there are no default values- is because @miszol1 used custom template, such as <storybook-button [label]="label">, in his storybook. The code and screenshot I've posted may confirm this is not the reason, since the default values are not set properly even if there is no custom template in the story file.

[tmeasday]

@marek-aguita can you upload your example somewhere so we can take a look? You can use https://storybook.new to create a simple reproduction..

[miszol1]

@tmeasday similar examples are provided in the original reproduction SB7 above.

[marek-aguita]

@tmeasday of course, here is the example:

https://stackblitz.com/edit/github-21hfon-ccgodj?file=src%2Fapp%2Fcomponents%2Finline-badge.component.html

as we can see, the control 'color' does not have the default value 'warn' set, despite the fact the component itself has it set properly:

[tmeasday]

@marek-aguita - OK, that's not a bug, that's what's intended to happen! We are seeing what happens when you don't provide a value for color to the component, so the control has no value selected.

I think if there is an issue here, it's the radio control perhaps could make it clearer that "this arg has no value" rather than right now where it arguably looks broken (although I could argue the other way, that it conveys that information well, 🤷). cc @cdedreuille.

I'll note additionally that there is another column in the table called "default value" that's visible in docs mode and can be enabled in the addons panel that would provide the information you wanted.

[marek-aguita]

@tmeasday but I am providing a value for color, maybe the stackblitz was not saved? Could you please verify you can see this code in the stackblitz?

I would expect that if the color property has explicitly set a valid value, in this case 'warn', the corresponding radio button will get selected by default, when the storybook page is opened or am I wrong?

[miszol1]

@miszol1 I'm not sure. I don't know a whole lot about angular templates but there may be a way to write them that doesn't rely on the default value being passed. Something like:

  // if label is not `undefined`
          <storybook-button [label]="label"></storybook-button>
  // otherwise
          <storybook-button></storybook-button>

Simply writing the first half is assuming (given the way that templates + defaults work) that label will always be set, which is not true (any more).

This looks like a React way of using component and its properties in html but not like in angular!
In this scenario i have to add weird ifs everywhere in my stories files? It does not make any sense :( For me it is still a regression bug. I can't use this version of storybook and i have to revert to SB6 because when i have 100+ components and stories for them it is impossible to maintain/change all of these stories.
I would like to remind that this feature worked in SB6.

[hettiger]

Had the same problem today. This really should be fixed in an upcoming release. For now I'm using the following workaround:

function attributes(object: object): string {
  return Object.entries(object).reduce(
    (attributes, [key, value]) => attributes + ` ${key}="${value}"`,
    ''
  );
}

const meta: Meta<LinkComponent> = {
  component: LinkComponent,
  render: (args) => ({
    props: args,
    template: `<a me-link ${attributes(args)}>Label</a>`,
  }),
};

I'm using ${key}="${value}" instead of [${key}]="${key}" because it improves the code snippets in the docs. There may be edge cases when this is not sufficient. Then you could always add e.g. an extra argument to the attributes function that accepts an array of keys where attributes should be bound to props using [${key}]="${key}".

I've moved the attributes function into stories/utils/index.ts and exported it from there.

[tmeasday]

@marek-aguita

I am providing a value for color, maybe the stackblitz was not saved?

You are providing a value yes, but not an arg value. The control displays the value of the arg (the input to the component), not the component's internal state. The idea is to show the user what would happen when the component is used with that input. In this case there's no input -- so it's actually helpful to show the user "if you don't supply the color input, here's what you'll see". IMO selecting warn is actually confusing because that's not being supplied to the component.

@miszol1 @hettiger

I would like to remind that this feature worked in SB6.
This really should be fixed in an upcoming release

I don't think this "bug" will get fixed. The reason we changed the behaviour was that it caused a lot of problems as it is all based on inferring the default value via static analysis tools (such as compodoc in this case). Those tools are imperfect and get the value wrong in some cases. That causes real problems in peoples' storybook's as components would error when passed the wrong value for an arg.

We didn't want to commit to 3rd party inference tools not having such bugs, and as you can see by my argument above, we also think this way makes more sense anyway.

It's unfortunate that (a) angular templates don't make it easy to have optional inputs (sort of begs the question why have default values in the first place?) (b) y'all have a lot of stories that relied on this behaviour. Perhaps we can find a workaround?

Are the default values available at runtime? Could you do something like:

export default: {
   component: LinkComponent,
   args: {
     ...LinkComponent.defaults
   }
}

If that works, you might even be able to the above automatically using an args enhancer. Or you could pull the statically extracted value argTypes.foo.defaultValue and use that ("at you own risk"). I can explain how to do that if it helps.

[marek-aguita]

@tmeasday so what you are basically saying in order to show the default value to the users of our Storybook we need to have both default value in the component, such as:

@Input() color: 'accent' | 'warn' = 'warn';

and also set the same value in args:

export const InlineBadge: StoryObj<InlineBadgeComponent> = {
  args: {
    color: 'warn'
  }
};

If yes, I also feel this is a downgrade since now I have to manually set args for all component properties that have some defaults. In SB6 this was really working well. I do hear your argument that compodoc is buggy and that is why this feature was dropped.

You are providing a value yes, but not an arg value. The control displays the value of the arg (the input to the component), not the component's internal state. The idea is to show the user what would happen when the component is used with that input. In this case there's no input -- so it's actually helpful to show the user "if you don't supply the color input, here's what you'll see". IMO selecting warn is actually confusing because that's not being supplied to the component.

Well, not really: I want to show to the user what will happen if they do not provide any value to the color Input: in that case it should be warn. However Storybook without the duplicated args: {color: 'warn'}} setting shows to the user falsy information: that the default value of the color property is actually undefined. I don't have any motivation to show this scenario to the user, because it cannot happen in a real application. I could even throw an error if the color property was explicitly set to undefined to ensure stronger certainty nobody will cause invalid state in the component.

I am interested in the workaround with argTypes.foo.defaultValue you mentioned, can you explain to us please?

[hettiger]

@tmeasday

@marek-aguita has explained it really well. The new behavior is really dangerous. It totally defeats the confidence that snapshot testing using e.g. chromatic could give you if you heavily rely on default values. That's because you must ensure that component default values and story default args are in sync at all times. That's why I've created my little workaround. However, I don't like that I have to do this. I also think, that the old behavior was much better.

[Marklb]

I think this issue's comments are mixing various different related issues.

The way inputs are handled, at the moment, is how I think it should be done, but I do see an issue explained below.

If we add the default value to args then it will get passed to props and Storybook is going to update that input value. Why should it do that? A default value does not need to be updated to the same value that it already is. In most scenarios that is not going actually to be an issue, but what if your component tracks if the input has been set?

If there is a value in props then Storybook is going to add that input to the generated snippet in "Show Code", but why show inputs being set to their default value? It isn't wrong to do that, but I don't want my developers copying that snippet and setting the input everywhere, because then there isn't really a point in having a default, since everyone is setting it to what the docs showed at the time.

I like how it currently works and would not want it to change, unless the functionality can be toggled, because I was actually frustrated by my defaults being in args for no reason.

I think the radio Control does show what I would probably consider a bug. The following screenshot is similar to what @marek-aguita posted above.
I am showing the component, controls, and devtools, where I logged args in the story's render function. inp1 is what I expect, but I would expect inp2 to be showing a button asking to set the value. To me, that "Set ..." button is like saying, "add this input and let me play with the value." inp3 is what I would expect.

Assuming the select showed a "Set ..." button, inp3 then presents the same problem, but in a way that I actually do consider a problem. Once I click "Set string", I am presented a text input, but I would expect my default value to be populated in that control.

Another problem, which @hettiger referred to, where providing a template does not make it simple to optionally add inputs to the component. He solved it by adding an attributes() function. I don't think that is what this issue is focused on, but it is something I have looked into before. I have a refactor that I wrote a while ago, before updating inputs that aren't explicitly set was removed. It fixes most of the issues that I think caused that to get removed, but I only focused on functionality, so it may not improve the snippet shown in "Show Code", but I will take a look. If it seems to fix that then I will share a link to try it out.

[tmeasday]

Thanks for commenting @Marklb. You are right there are two issues and I think they are getting mixed up a bit. We can discuss them both but let's be clear about what we are talking about:

SB not showing default value in input control

Let's be really clear. There are two things:

1. The arg which is the input provided to the component
2. The components internal state which is set to the default value if no input is provided.

The control shows the arg's value. The arg does not automatically get set to the default value at any stage, so it doesn't make sense to put the default value in the input. This is behaviour that is non-Angular specific and is the same across the entirety of Storybook. You can find tickets where React/etc users complain about the same thing, but this is the behaviour we have settled on.

but I would expect inp2 to be showing a button asking to set the value.

I agree with this, it is inconsistent that other unset/undefined controls show a button but the radio renders with no value selected. We'd accept a PR changing this.

inp3 then presents the same problem, but in a way that I actually do consider a problem. Once I click "Set string", I am presented a text input, but I would expect my default value to be populated in that control.

This is where we disagree. Again, by the argument above, the arg is not getting the default value, so setting it to the default value is confusing. It might make sense to show a placeholder on the input with the default value? That won't work for the radio however.

This is a good time for me to mention again, it's possible to show the default value in a separate column, using the controls.expanded parameter:

Angular templates behaving "badly" when an expected input is not passed

It sounds like maybe this is something @Marklb is looking at. @valentinpalkovic do you know anything about this?

This is the area I am less sure about, so I won't stick my foot in it and comment further :)

---

In response to some other questions:

@marek-aguita

what you are basically saying in order to show the default value to the users of our Storybook we need to have both default value in the component

What do you mean by "show"? If you mean render in the component, then no, just don't set the arg and the component will render the default value (modolo the template issue of course). If you mean show in the controls addon/in docs, that's why we have the "default value" column, see above.

However Storybook without the duplicated args: {color: 'warn'}} setting shows to the user falsy information: that the default value of the color property is actually undefined.

This sounds like the template issue above? Like I said, I think this is a different problem with likely a different solution. We can discuss here or on a separate ticket.

@hettiger

The new behavior is really dangerous. It totally defeats the confidence that snapshot testing using e.g. chromatic could give you if you heavily rely on default values.

Again, is this due to the templating problem?

In my mind, setting the arg to the default value automatically (and not actually exercising the default value at all) via an unreliable tool like compodoc would give me less confidence than just not setting the value at all and letting the default apply. Don't you agree?

---

@marek-aguita

I am interested in the workaround with argTypes.foo.defaultValue you mentioned, can you explain to us please?

So my first question is do we have runtime information from Angular about what the defaults are? If so it'd be better to use that than the compodoc-inferred defaults (argTypes.foo.table.defaultValue.summary).

Either way, once you have a value to use as the default, you'd use an argsEnhancer to set the arg from the default value (if unset I guess). Something like (in preview.ts):

export const argsEnhancers = [({ args, argTypes }) =>
  Object.fromEntries(
    Object.entries(argTypes).map(([argName, { table: { defaultValue: { summary } } } ]) =>
      [argName, args[argName] ?? summary]
    )
  }),
});

[Marklb]

@tmeasday I was thinking the default values in props was more specific to Angular than it is. So, based on that clarification, I agree that my expectation about the default populating, when clicking the "Set ..." button, is wrong.

[valentinpalkovic]

As @tmeasday already mentioned, let's focus on the Angular templates behaving "badly" when an expected input is not passed issue, since the topic SB not showing default value in input control seems for me to be discussed enough in the past and we have decided to go that route.

Angular templates behaving "badly" when an expected input is not passed

The reason why this feels "bad" is how Angular deals with passed null or undefined values. They are not replaced by their defaults in Angular. This is indeed the intended behavior, although it doesn't match how the rest of the Javascript/Typescript ecosystems deal with defaults and passed undefined arguments/properties. As soon as a value is passed to an Angular component, its value is taken, regardless of whether it is undefined.

There is a feature request that tries to implement a new kind of data binding, where indeed, the default is taken every time undefined is passed.

Another way is to handle the undefined value in ngOnChanges to fallback to your default value. But I don't think that the Angular community wants to configure ngOnChanges on every single component, just to make Storybook work properly.

Workaround

This all means, that in Storybook we are not allowed to bind a value, if the value is actually undefined. We could use a helper function like this to circumvent the issue:

function mapArgsToBindings(args: Record<string, any>) {
  return Object.entries(args)
    .flatMap(([key, value]) => {
      if (value !== undefined) {
        return [`[${key}]="${key}"`];
      } else {
        return [];
      }
    })
    .join(' ');
}

export const Template: Story = {
  render: (args) => ({
    props: args,
    template: `<app-example ${mapArgsToBindings(args)}></app-example>`,
  }),
  args: {
    label2: 'story label2',
  },
};

Every time an argument is undefined, it doesn't get bound to the component, and therefore the default value gets applied. I don't know, which kind of other implications we would have with a solution like this.

Possible fixes

In Storybook, we could provide the users a "magic string" slice like $args, which got internally replaced by the "patched" property binding. So the user might define a Story like this:

export const Template: Story = {
  render: (args) => ({
    props: args,
    template: `<app-example $args></app-example>`,
  }),
  args: {
    label2: 'story label2',
  },
};

In the background, we replace $args with the property binding, which was showcased in the workaround section.

@tmeasday WDYT?

[tmeasday]

Seems sensible to me. Would people sometimes want to pass some of the args to one place and some to another?

[valentinpalkovic]

I assume there are some scenarios where this is the case. But I would say that in principle, $args might be a day-to-day helper, which helps in everyday scenarios where you use the template function to apply some additional wrappers, and you are bored applying all args all the time + having the issues, which are raised by non-applying default values.

[hettiger]

I think the proposed $args magic string goes in the right direction. However, the args parameter may contain key : value pairs that are not intended to be passed to component @input's: https://storybook.js.org/docs/angular/writing-stories/args#args-can-modify-any-aspect-of-your-component

I'm not really sure what the props are in this context. I guess the props define the context / available variables for the template? So these probably have the same problem:

<app-example [propA]="propA"><child-example [propB]="propB"></child-example></app-example>

You don't want the magic string $args to bind [propB]="probB" to the <app-example> component in this case…

Maybe Storybook could just provide a few exported functions:

attributes(args)
omit(args, keys)
only(args, keys)

Then you can just do something like this:

const meta: Meta<LinkComponent> = {
  component: LinkComponent,
  render: (args) => ({
    props: args,
    template: `<a me-link ${attributes(omit(args, ['propB']))}>Label</a>`,
  }),
};

The magic string solution could also be offered. It could simply be the equivalent of:

const meta: Meta<LinkComponent> = {
  component: LinkComponent,
  render: (args) => ({
    props: args,
    template: `<a me-link ${attributes(args)}>Label</a>`,
  }),
};

Of course the magic string solution would just use the attributes function internally…

If you really want to get fancy you could add parameters to the magic string. E.g. $args:propB,propC. However, I think this goes too far.

[hettiger]

@hettiger

The new behavior is really dangerous. It totally defeats the confidence that snapshot testing using e.g. chromatic could give you if you heavily rely on default values.

Again, is this due to the templating problem?

Yes

In my mind, setting the arg to the default value automatically (and not actually exercising the default value at all) via an unreliable tool like compodoc would give me less confidence than just not setting the value at all and letting the default apply. Don't you agree?

Totally agree. Solving the templating problem is key here I think.

[valentinpalkovic]

attributes(args)
omit(args, keys)
only(args, keys)

I like this suggestion as well!

If we want to support these additional two use cases, we could also provide one helper function via a callback argument. Like this:

const meta: Meta<LinkComponent> = {
  component: LinkComponent,
  render: (args, {attr}) => ({
    props: args,
    template: `<a me-link ${attr()}>Label</a>`,
  }),
};

attr's signature could be something like this:

interface Props<K> {
   include?: Array<K>;
   exclude?: Array<K>;
}

function attr<K extends string[]>(props: Props<K>) {
  ...
}

So we could even define some nice types for include and exclude, which would match the arguments types extracted from the component definition.

We wouldn't need to pass args additionally into it since the closure might already be aware of it.

[tmeasday]

This is the CSF-defined render function right @valentinpalkovic? I agree putting attrs on the context seems like a nice API, I do think there are some technical challenges in getting it on there / typing it right. Maybe we can figure it out though.

[valentinpalkovic]

@hettiger, @Marklb, @marek-aguita

I have released a snapshot release of this PR:
0.0.0-pr-24434-sha-0d375607

I have introduced a new utility function, argsToTemplate, designed to assist in generating Angular component templates for Storybook stories. Could you try it out by installing the snapshot version of Storybook for all Storybook packages?

The usage is pretty simple:

import { Meta, StoryObj, argsToTemplate } from '@storybook/angular';

...

export const WithTemplate: Story = {
  args: { label: 'Template test', appearance: 'primary' },
  render: (args) => ({
    props: args,
    template: `<my-button ${argsToTemplate(args)}></my-button>`,
  }),
};

As a second argument, argsToTemplate accepts an include/exclude option for further customization. Would you like to give me some feedback on whether this satisfies your needs?

@tmeasday

After talking to @shilman and @ndelangen we have concluded to not provide the utility function via context parameter within the render function, but instead just provide an exported utility function from @storybook/angular.

[hettiger]

Thank you @valentinpalkovic this seems to work great for me so far. If I run into any issues I'll let you know.