Angular: Overwriting Input/Output properties of base component leads to incomplete metadata

[stefan-schweiger]

Describe the bug
StorybookWrapperComponent does not correctly get @Input and @Output properties from an inherited parent component. This can lead to them getting overwritten and behaving unexpectedly.

As far as I was able to recreate this issue it's actually a bug in how Angular handles it's metadata. Since I don't have high hopes that this gets fixed anytime soon in Angular is there interest in fixing this via a workaround in storybook?

I have mitigated this issue in my local branch by iterating and merging the parent prop metadata:

export const getComponentPropsDecoratorMetadata = (component: any) => {
  const decoratorKey = '__prop__metadata__';
  let propsDecorators: Record<string, (Input | Output)[]> =
    Reflect &&
    Reflect.getOwnPropertyDescriptor &&
    Reflect.getOwnPropertyDescriptor(component, decoratorKey)
      ? Reflect.getOwnPropertyDescriptor(component, decoratorKey).value
      : component[decoratorKey];

  // START WORKAROUND
  const parent = Reflect && Reflect.getPrototypeOf && Reflect.getPrototypeOf(component);

  if (parent) {
    const parentPropsDecorators = getComponentPropsDecoratorMetadata(parent);

    propsDecorators = { ...parentPropsDecorators, ...propsDecorators };
  }
  // END WORKAROUND

  return propsDecorators;
};

Please let me know if we should go down this route then I will create a PR with my fix.

To Reproduce

Add these two files anywhere in the angular example in the repo:

test.component.ts

// eslint-disable-next-line max-classes-per-file
import { Component, EventEmitter, Input, Output } from '@angular/core';

@Component({
  selector: 'hadv-storybook-test',
  template: '',
})
export class BaseTestComponent {
  private _value?: string;

  @Input()
  public get value(): string | undefined {
    return this._value;
  }

  public set value(value: string | undefined) {
    this.writeValue(value);
  }

  @Output()
  public get valueChange(): EventEmitter<string | undefined> {
    return this.#valueChange;
  }

  #valueChange = new EventEmitter<string | undefined>();

  public writeValue(value: string | undefined) {
    this._value = value;
    this.valueChange.emit(value);
  }
}

@Component({
  selector: 'storybook-test',
  template: '{{ value }} <button (click)="changeValue()">Set Values</button>',
})
export class TestComponent extends BaseTestComponent {
  @Input()
  public get value(): string | undefined {
    return `${super.value} test`;
  }

  public set value(value: string | undefined) {
    super.value = value;
  }

  public changeValue(): void {
    this.value = (Math.random() * 100).toFixed(0);
  }
}

test.component.stories.ts

import { Story } from '@storybook/angular';

import { TestComponent } from './test.component';

export default {
  title: 'Base/Test',
  component: TestComponent,
  argTypes: { valueChange: { action: 'valueChange' } },
};

export const Base: Story = (args: any) => ({
  props: args,
  template: `
    <storybook-test value="${args.value}"></storybook-test>
  `,
});

Base.args = {
  value: '2',
};

System
Please paste the results of npx sb@next info here.

Additional context
Add any other context about the problem here.

[stefan-schweiger]

Since the Angular folks say that this works as intended from their end I've created a pull request to fix the incomplete metadata on the storybook end.

[Olusha]

@stefan-schweiger, I have updated to the latest @6.4.0-alpha.22 version, but the input parameters setter function is called after ngAfterInit hook is there are parameters in the child component that is not aligned with usual angular behaviour.

Parent

export abstract class AbstractMainComponent implements AfterViewInit {
  @Input() getLabel: () => string;

  ngAfterViewInit(): void {
    // getLabel is not initialized here 
    console.log('ngAfterViewInit', this.getLabel);
  }
}

Child

  selector: 'app-abstract',
  templateUrl: './abstract.component.html',
  styleUrls: ['./abstract.component.css']
})
export class AbstractComponent extends AbstractMainComponent implements AfterViewInit {
  @Input() ownInput: string;

  constructor() {
    super();
  }

  loadLabel(): void {
// getLabel is not initialized 
    console.log('LABEL', this.getLabel);
  }
}

Story

  title: 'Example/Inheritance',
  component: AbstractComponent,
  imports: [CommonModule],
} as Meta;

const Template: Story<AbstractComponent> = (args: AbstractComponent) => ({
  component: AbstractComponent,
  props: args,
});

export const Primary = Template.bind({});
Primary.args = {
  getLabel: () => 'super test'
};

[stefan-schweiger]

@Olusha can you create a reproduction project for me?

If you add the property to the child component do you get the same behavior? If yes, this works as intended and was "broken" before because of the missing meta data.

[Olusha]

@stefan-schweiger , please, check if it is available https://stackblitz.com/edit/angular-ivy-mtpvba?file=src/app/child/child.component.ts
https://github.com/Olusha/angular-storybook-inheritance

If a storybook is added and a story for the child component is created then the parent's input property is not initialized in the ngAfterViewInit (it gets initialized after some time, maybe lifecycle hooks) if the child has its own input property, but if the child has no input parameters then everything works as expected

[stefan-schweiger]

@Olusha were in your code do you initialize getLabel? It is always undefined in your example even without storybook for me.

[Olusha]

@stefan-schweiger , that is because the function was not provided as a directive

Now updated https://stackblitz.com/edit/angular-ivy-mtpvba?file=src/app/child/child.component.ts

[stefan-schweiger]

@Olusha ok I've now tried to recreate your issue with this story:

child.component.stories.ts

import { AfterViewInit, Component, Directive, Input } from "@angular/core";
import { Story, Meta, ArgTypes } from "@storybook/angular";

@Directive()
abstract class AbstractParent implements AfterViewInit {
  @Input() getLabel: () => string;

  ngAfterViewInit(): void {
    // getLabel is not initialized here if running storybook
    console.log("ngAfterViewInit", this.getLabel && this.getLabel());
    this.loadLabel();
  }

  abstract loadLabel(): void;
}

@Component({
  selector: "app-child",
  template: `childInput: {{ childInput }}<br />
    getLabel: {{ getLabel && getLabel() }}<br />
    getLabelChild: {{ getLabelChild && getLabelChild() }}<br /> `,
})
class ChildComponent extends AbstractParent implements AfterViewInit {
  @Input() childInput: string;

  @Input() getLabelChild: () => string;

  constructor() {
    super();
  }

  loadLabel(): void {
    // getLabel is not initialized here if running storybook
    console.log("LABEL", this.getLabel && this.getLabel());
    console.log("LABEL", this.getLabelChild && this.getLabelChild());
  }
}

export default {
  title: "Example/ChildComponent",
  component: ChildComponent,
} as Meta;

export const Base: Story<ChildComponent> = (args: ChildComponent) => ({
  props: args,
});

Base.args = {
  getLabel: () => 'Test',
  getLabelChild: () => 'Test 2'
};

But the behavior is exactly the same with 6.3 and 6.4-alpha.22. So my PR didn't fix your specific issue but at least it didn't cause it.

It's pretty easy to get the storybook source code running and trying to debug the issue yourself. I'm not a member of the team but PRs are always welcome. Else I would suggest creating a separate bug report, as I think those issues are at least a bit different.

[shilman]

Yo-ho-ho!! I just released https://github.com/storybookjs/storybook/releases/tag/v6.4.0-alpha.23 containing PR #15586 that references this issue. Upgrade today to the @next NPM tag to try it out!

npx sb upgrade --prerelease

Closing this issue. Please re-open if you think there's still more to do.

[Olusha]

@shilman, I have upgraded storybook
but now have the following error

[shilman]

@Olusha I wonder if that's due to this PR that was merged: #15298

Does it fix it if you pin your version of prettier to ~2.2.1

cc @ndelangen

[ndelangen]

What the hell is that syntax?

[shilman]

@ndelangen allows patch level improvements (e.g. 2.2.x). see https://www.npmjs.com/package/semver

not advocating for it, but it's what we moved away from and appears to have possibly borked something in @Olusha 's project

[Olusha]

@shilman , as far as I understood, @stefan-schweiger 's fix (PR #15586 ) didn't fix my specific problem.
Also I found one more problem with an inheritance: let's add input childFn to the child component of type function (https://stackblitz.com/edit/angular-ivy-lo5uhk?file=src/app/child/child.component.ts). Then if I try to provide the function, that returns an object, in the template in the story, I have a compilation error.
Story example:

  component: AbstractComponent,
  props: args,
  template: `<app-abstract [getLabel]="getLabel" [ownFn]="ownFn"></app-abstract>`
});

export const Primary = Template.bind({});
Primary.args = {
  getLabel: () => of('getLabel Fn'),
  ownFn: () => of( 'ownFn'),
};

Error:

[stefan-schweiger]

@Olusha @shilman after updating to 6.5-alpha.23 the reproduction for @Olusha that I've posted here is now working correctly:

So I guess it was actually the same issue. Why you are getting the new errors I don't know. At least my repro works without a problem. Can you maybe send us a minimal reproduction?

[Olusha]

@shilman , yes, my first issue with Input property inheritance is fixed in the 6.4-alpha.23, but the second issue I described is still there.
The problem occurs if I try to send a function that returns the object as an arg to the story.
If primitive value is return by the function, everything works as expected.
Link to the sandbox (https://stackblitz.com/edit/angular-ivy-lo5uhk?file=src/app/child/child.component.ts)

  component: AbstractComponent,
  props: args,
});

export const Primary = Template.bind({});
Primary.args = {
  getLabel: () => {
    return  of(1);
  },
  ownFn: () => {
    return  of(1);
  },
};