Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Storybook 6.2.0-beta new angular renderer ovverides properties #14147

Closed
kbrilla opened this issue Mar 5, 2021 · 9 comments
Closed

Storybook 6.2.0-beta new angular renderer ovverides properties #14147

kbrilla opened this issue Mar 5, 2021 · 9 comments

Comments

@kbrilla
Copy link

kbrilla commented Mar 5, 2021

Describe the bug
if component has some properties that are not inputs they are overridden

export class SampleComp{
   @Input() input1: string;
   notInputProperty: string;

  ngOnChanges(){
  this.notInputProperty = 'kokos';
   }
}

and template will be

notInputProperty : {{notInputProperty }}

then notInputProperty will be overridden after change detection we will get rendered ```notInputProperty : `.

if we add @Input decorator to this property then it works as it should and we get rendered: ```notInputProperty : kokos'.

setting angularLegacyRendering: true, solves the issue.

export default {
  title: 'Components/Core/SampleComp',
  component: SampleComp,
  decorators: [
    moduleMetadata({
      declarations: [SampleComp],
    }),
  ],
} as Meta;

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

export const Sample = Template.bind({});
Sample.args = {};

fun fact:
after first time storybooks renders You will change the name of property to lets say notInputProperty2 then it will work!
So maybe it has something to do with controls?
Restarting storybook (ctr+c) and npm run storybook again will again cause it to be overridden till You again change the name of property to lets say notInputProperty3.

System

  System:
    OS: Windows 10 10.0.19042
    CPU: (8) x64 Intel(R) Core(TM) i5-8265U CPU @ 1.60GHz
  Binaries:
    Node: 14.16.0 - C:\Program Files\nodejs\node.EXE
    npm: 6.14.11 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Chrome: 88.0.4324.190
    Edge: Spartan (44.19041.423.0), Chromium (88.0.705.81)
  npmPackages:
    @storybook/addon-actions: 6.2.0-beta.10 => 6.2.0-beta.10
    @storybook/addon-centered: 5.3.21 => 5.3.21
    @storybook/addon-essentials: 6.2.0-beta.10 => 6.2.0-beta.10
    @storybook/addon-links: 6.2.0-beta.10 => 6.2.0-beta.10
    @storybook/addon-storysource: 6.2.0-beta.10 => 6.2.0-beta.10
    @storybook/angular: 6.2.0-beta.10 => 6.2.0-beta.10
@ThibaudAV
Copy link
Contributor

ThibaudAV commented Mar 10, 2021

Yes, it's true. But I don't think it's a "bug". 🙈 (but a change in behaviour that I would also like to make). Give me a few days to find time to make a complete answer. (very busy lately 😞 )

@kbrilla
Copy link
Author

kbrilla commented Mar 10, 2021 via email

@ThibaudAV
Copy link
Contributor

ThibaudAV commented Mar 15, 2021

In fact I don't know where to start... many things are mixed up

Problem Replication:

If I understand correctly your example is this:

Click to expand!
import { Story, Meta, moduleMetadata } from '@storybook/angular';
import { Input, Component } from '@angular/core';

@Component({ selector: 'foo', template: 'notInputProperty : {{notInputProperty }}' })
class SampleComp {
  @Input() input1: string;

  notInputProperty = 'rr';

  ngOnChanges() {
    this.notInputProperty = 'kokos';
  }
}

export default {
  title: 'Legacy / angularLegacyRendering',
  component: SampleComp,
  decorators: [
    moduleMetadata({
      declarations: [SampleComp],
    }),
  ],
} as Meta;

const Template: Story<SampleComp> = (args) => {
  console.log(args);

  return {
    props: args,
    // angularLegacyRendering: true,
  };
};

export const Sample = Template.bind({});
Sample.args = {};

here are the steps I do :

  • Add story without angularLegacyRendering: true
  • run yarn storybook

see the console.log {input1: undefined, notInputProperty: undefined, ngOnChanges: undefined}

  • change input1 value with controls

nothing happen but i have log + error in console :
image

  • change with angularLegacyRendering: true

see the console.log {} :wow: empty now

  • change input1 value with controls

see the console.log {input1: "s"}
It works, we can see Koko in canvas

Now, if i do same step (close storybook client and restart it) but i start storybook with angularLegacyRendering: true

I have the same behavior.
So, If it's the same problem you encountered, the problem is not the new rendering
I'm not mistaken? 🙏

I think the problem comes from compodoc. Which discovers all properties of your component class and adds them to agrs.
If you don't control it, it will do anything.

Discussions are already in progress to replace compodoc
#12689
#8672

personally I don't like compodoc at all and I don't use it

I recommend you not to trust compodoc if you use it and to do something like :

const Template: Story<SampleComp> = (args) => ({
  props: {
  input1: args.input1
  },
});

to be sure to send only the expected "arg"


So :

  • compodoc reference private properties

Wait for an alternative. or manually add controls of args

  • props can override private (or non @input) properties of components

Several topics are talking about it, it also blocks some features
I have already made a response in some of them

#11613
#12946

or else #12438

I really wish I could ban it or split it into 2 like:
props : only angular Input / Output / directive (ngClass,...) or html attributs (class,...)
overrideComponentProperties: As before allows to override a property of the component class

but this is a big breaking changes I think and we have to wait for a major version (I was told) 😁


fun fact :

fun fact:
after first time storybooks renders You will change the name of property to lets say notInputProperty2 then it will work!
So maybe it has something to do with controls?

not with controls with compodoc. As you do not completely reload the storybook client compodoc does not reanalyze your component


legacy rendering and ngOnChanges

I don't recommend to use legacy rendering if you have stories and component with ngOnchanges. it is manually called by the renderer and is not exactly the same as native angular.
see:

The new renderer doesn't work the same way because it simulates a template for your component and therefore lets the native angular mechanism do the work

@kbrilla kbrilla closed this as completed Mar 24, 2021
@kbrilla kbrilla reopened this Mar 24, 2021
@nicolae536
Copy link

nicolae536 commented Apr 1, 2021

A possible fix until the main issue gets fixed you can disable compodoc in main.js. But as a result you have to declare the argTypes on all components and maintain it.

// Because of the following issues we cannot use Compodoc to automatically find component inputs:
// https://github.com/storybookjs/storybook/issues/14147
// https://github.com/storybookjs/storybook/issues/11613
// It seems storybook will try to add support for detecting inputs because compodoc is not maintained anymore and they end up overriding angular component
// props which are not imputs with ng versions bigger than 8.0
// When this is closed we can probably see what's the upgrade path: https://github.com/storybookjs/storybook/issues/8672
// import { setCompodocJson } from "@storybook/addon-docs/angular";
import { INITIAL_VIEWPORTS } from '@storybook/addon-viewport';
// import docJson from "../documentation.json";
// setCompodocJson(docJson);


export const parameters = {
  actions: { argTypesRegex: "^on[A-Z].*" },
  controls: {
    matchers: {
      color: /(background|color)$/i,
      date: /Date$/,
    },
  },
  viewport: {
    viewports: INITIAL_VIEWPORTS,
  },
}

@salmin89
Copy link

salmin89 commented Apr 15, 2021

@ThibaudAV

legacy rendering and ngOnChanges

I don't recommend to use legacy rendering if you have stories and component with ngOnchanges. it is manually called by the renderer and is not exactly the same as native angular.
see:

We turned on legacyRendering because we got these kinds of errors:

ERROR TypeError: provider.ngAfterViewInit is not a function

How would we turn off angularLegacyRendering and solving the above error?

@ThibaudAV
Copy link
Contributor

@salmin89 can you give more details about the component you added in the story ?

Make sure that you do not directly pass all args in your story props
prefer a "destructuring" to control them :

const Template: Story<SampleComp> = (args) => ({
  props: {
  input1: args.input1
  },
});

@ThibaudAV
Copy link
Contributor

I think we can close it a workaround has been added to avoid the override
can you check if the problem is still present with the last version 🙏

@shilman shilman closed this as completed Jun 3, 2021
@DigiBanks99
Copy link

DigiBanks99 commented Jan 13, 2022

Hi, the problem still persists and the workaround also doesn't work anymore.

Storybook: v6.50-alpha.14
Compodoc: v1.1.18
Angular: v13

@pscadding
Copy link

pscadding commented Jun 8, 2022

In case it helps anyone else who ends up here I added --disablePrivate, --disableInternal and --disableLifeCycleHooks to the storybook builder compodocArgs options to get it to stop finding the private properties.
Added in the angular.json:

{
  ...
  "projects": {
    "my-lib": {
      ...
      "architect": {
        "storybook": {
          "builder": "@storybook/angular:start-storybook",
          "options": {
            ...
            "compodocArgs": ["-e", "json", "--disablePrivate", "--disableInternal", "--disableLifeCycleHooks"]
          }
        },

Another workaround that I don't like as much is to set the parameters on my story to either exclude or include the properties from the controls.

Primary.parameters = {
  controls: {
    // either exclude or include
    exclude: ['ngAfterContentInit', 'display'],
    include: ['percentage', 'error', 'status']
  },
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

7 participants