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

Issue with Angular and Storybook #14761

Closed
jonniebigodes opened this issue Apr 29, 2021 · 6 comments
Closed

Issue with Angular and Storybook #14761

jonniebigodes opened this issue Apr 29, 2021 · 6 comments

Comments

@jonniebigodes
Copy link
Contributor

Describe the bug
While updating the Intro to Storybook tutorial to reflect the changes introduced in Storybook 6.2 I came across a bit of a weird issue on how Storybook is displaying the stories incorrectly. Following the steps provided in the Composite Component chapter, starting Storybook me or any other reader should get the following Storybook state. But in reality, what I got was:

storybook-angular-issue-opt.mp4

The Tasklist (default and with pinned tasks) should have rendered the stories correctly but instead, they're showing that there's nothing. Basically treating it as it was empty.

To Reproduce

  • Clone this repo
  • Install dependencies
  • Run npm run storybook
  • In .storybook/preview.js comment out angularLegacyRendering: true,
  • Start Storybook with `npm run start-storybook``
  • Check the Default and With Pinned story under TaskList.

@ThibaudAV could you take a look please and let me know what might be the issue here. As a safeguard, I adjusted the tutorial to mention to the readers that they should add the angularLegacyRendering: true to their preview.js in order to get the correct stories showing.

Thanks in advance

@ThibaudAV
Copy link
Contributor

yes i'll have a look ;)

@jonniebigodes
Copy link
Contributor Author

Really appreciate it 🙏🏼! Let me know of your findings.

@ThibaudAV
Copy link
Contributor

it's the same problem as here with workaround #14732 (comment)
I have the impression that 80% of the angular issues are on this problem 😢

I do not know how to fix in any case it is a breaking changes 🤣


You have this private props tasksInOrder that will follow these steps

  1. is added in the doc of compodoc
  2. then in argTypes
  3. then in args with defaultValue found by compodoc ([] in your case)
  4. and finally Angular allows the override private properties
    💥

If you make the same test by changing the order of the props you will have the same problem
try this :
in pure-task-list.stories.ts

const Template: Story<PureTaskListComponent> = ({ tasks, ...args }: any) => {
  console.log(args);

  return {
    props: {
      tasks: args.tasks,
      ...args,
      onPinTask: TaskStories.actionsData.onPinTask,
      onArchiveTask: TaskStories.actionsData.onArchiveTask,
    },
    //template: `<app-pure-task-list [tasks]="tasks" [loading]="loading" (onPinTask)="onPinTask($event)" (onArchiveTask)="onArchiveTask($event)"></app-pure-task-list>`,
  };
};

you will have the same problem whatever the value of angularLegacyRendering

so between legacy and non-legacy renderer does not process props in the same order
and even more it depends on the property order of the args object

which is comical ; is that the new renderer uses the Input/Output like a real application. so no more problem of property order
but as it keeps the ability to override the private properties it prevents a correct working


there are several problems already discussed in other issus with @shilman

IMHO the main ones are :

  • argTypes should be able to display some doc without adding a value in args

  • Angular should not override private properties by default. but do it only if explicitly requested
    (And maybe also change the order of assignment of properties with props)


Sources ((so much magic in these lines of code )):
old :

if (!(instanceProperty instanceof EventEmitter) && value !== undefined && value !== null) {
// eslint-disable-next-line no-param-reassign
instance[key] = value;

new:
Object.assign(this.storyComponentElementRef, props);

(this.storyComponentElementRef as any)[p] = initialProps[p];

@shilman
Copy link
Member

shilman commented Apr 30, 2021

@ThibaudAV if you have time today let's meet and talk this through and fix it.

@jonniebigodes
Copy link
Contributor Author

@ThibaudAV thank you very much for the help on this and for pointing me in the right direction 🙏🏼. I was able to address the issue with the issue you've mentioned. And just merged an update for the tutorial with this. Really appreciate it.

@ThibaudAV
Copy link
Contributor

can we close this issue ? :)

@shilman shilman closed this as completed Jun 3, 2021
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

3 participants