storybookjs / storybook

Storybook is the industry standard workshop for building, documenting, and testing UI components in isolation
https://storybook.js.org
MIT License
84.68k stars 9.32k forks source link

Addons: error Converting circular structure to JSON for Angular mat-table #16855

Open Tatsianacs opened 2 years ago

Tatsianacs commented 2 years ago

Describe the bug When MatTableDataSource is used in addons arguments, the story is broken

const dataSource = new MatTableDataSource(ELEMENT_DATA);

export const Primary = Template.bind({});
Primary.args = {
  testDataSource: dataSource
}
vendors~main.a935f15a2179a6eff5fd.manager.bundle.js:2 TypeError: Converting circular structure to JSON
    --> starting at object with constructor 'MapSubscriber'
    |     property '_parentOrParents' -> object with constructor 'Subscriber'
    |     property '_subscriptions' -> object with constructor 'Array'
    --- index 0 closes the circle
    at JSON.stringify (<anonymous>)
    at ObjectControl (main.991fa6e71cce7c92d381.manager.bundle.js:1)
    at oh (vendors~main.a935f15a2179a6eff5fd.manager.bundle.js:2)
    at Rj (vendors~main.a935f15a2179a6eff5fd.manager.bundle.js:2)
    at Qj (vendors~main.a935f15a2179a6eff5fd.manager.bundle.js:2)
    at Kj (vendors~main.a935f15a2179a6eff5fd.manager.bundle.js:2)
    at yj (vendors~main.a935f15a2179a6eff5fd.manager.bundle.js:2)
    at vendors~main.a935f15a2179a6eff5fd.manager.bundle.js:2
    at exports.unstable_runWithPriority (vendors~main.a935f15a2179a6eff5fd.manager.bundle.js:2)
    at cg (vendors~main.a935f15a2179a6eff5fd.manager.bundle.js:2)

To Reproduce

  1. Generate angular component with Angular material table <table mat-table [dataSource]="dataSource">
  2. Create a story for the component where args will contain a data for mat-table dataSource as new MatTableDataSource(ELEMENT_DATA) => story load is broken

Repo: https://github.com/Tatsianacs/storybook--bug

System Environment Info:

System: OS: Windows Server 2016 10.0.14393 CPU: (6) x64 Intel(R) Core(TM) i7-5820K CPU @ 3.30GHz Binaries: Node: 16.13.0 - C:\Program Files\nodejs\node.EXE Yarn: 3.1.1 - ~\node_modules.bin\yarn.CMD npm: 8.1.0 - C:\Program Files\nodejs\npm.CMD Browsers: Chrome: 95.0.4638.69 npmPackages: @storybook/addon-actions: ^6.4.2 => 6.4.2 @storybook/addon-essentials: ^6.4.2 => 6.4.2 @storybook/addon-links: ^6.4.2 => 6.4.2 @storybook/angular: ^6.4.2 => 6.4.2 @storybook/builder-webpack5: ^6.4.2 => 6.4.2 @storybook/manager-webpack5: ^6.4.2 => 6.4.2

Additional context It doesn't work at least for Controls and accessibility addons, it worked for Knobs and nothing is broken if addon is disabled OR dataSource is removed from the table (workaround:

dataSource: {
            table: {
                disable: true,
            }
        }
shilman commented 2 years ago

Args must be JSON-serializable (ish). To use complex structures, we have a mapping construct https://storybook.js.org/docs/react/writing-stories/args#mapping-to-complex-arg-values

Tatsianacs commented 2 years ago

@shilman could you please provide an example of mapping for my case?

shilman commented 2 years ago

Untested:

const dataSource = new MatTableDataSource(ELEMENT_DATA);

export default {
  title: 'foo',
  argTypes: {
    testDataSource: {
      mapping: { Default: dataSource }
    }
  }
}
export const Primary = Template.bind({});
Primary.args = {
  testDataSource: 'Default'
}
Tatsianacs commented 2 years ago

@shilman it helped, thank you, should I close the issue or any fix is expected (e.g. provide a message in addon about required mapping instead of story crash?)

shilman commented 2 years ago

Helpful error message is a great idea. I'll follow up, thanks!! 🙏

stale[bot] commented 2 years ago

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

pbrianmackey commented 2 years ago

Untested:

const dataSource = new MatTableDataSource(ELEMENT_DATA);

export default {
  title: 'foo',
  argTypes: {
    testDataSource: {
      mapping: { Default: dataSource }
    }
  }
}
export const Primary = Template.bind({});
Primary.args = {
  testDataSource: 'Default'
}

The @shilman pattern also works for Angular Reactive Forms

R-Lek commented 1 year ago

The @shilman pattern also works for Angular Reactive Forms

Edit/ I was doing it wrong. I think I'm correctly using the @shilman pattern now in Storybook 7, instantiating the new FormControls outside of argTypes. However, while switching from 'Marty' to 'Jennifer' initially causes no problems, but then switching back to 'Marty' again gives the Converting circular structure error again:

const marty = new FormControl({ value: 'Marty McFly', disabled: false }, [
  Validators.required,
]);

const jennifer = new FormControl({ value: 'Jennifer Parker', disabled: true }, [
  Validators.required,
]);

const meta: Meta<InputFieldComponent> = {
  title: 'Components/Input/InputFieldComponent',
  component: InputFieldComponent,
  argTypes: {
    control: {
      options: ['Marty', 'Jennifer'],
      mapping: {
        Marty: marty,
        Jennifer: jennifer,
      },
      control: {
        type: 'select',
      },
    },
  },
}; 
valentinpalkovic commented 1 year ago

After having a chat internally, we figured out a workaround: https://stackblitz.com/edit/github-3znyoe-aebjsf?file=src%2Fstories%2Finput.component.stories.ts

So instead of using the native mapping functionality of Storybook, you can use the render property to do the mapping manually:

...
const mapping = {...};

const meta: Meta<InputField> = {
  ...
  argTypes: {
    control: {
      options: ['Marty', 'Jennifer', 'Doc'],
      control: {
        type: 'select',
      },
    },
  },
  render: ({ control }: {control: any}) => {
    const controlKey = control as keyof typeof mapping
    return ({
      props: {
        control: mapping[controlKey]?.(),
      },
    })
  },
};
...

The main issue is that the FormControl instances get only instantiated once. Even if you switch the control values in the ArgsTable for control, the same FormControl instance is used, which was instantiated initially. A new instance is NOT created. That's a limitation of the current implementation of Storybook's mapping capabilities. It seems that as soon as an input element gets rendered, the FormControl instance changes internally and creates a self-reference. If you now switch from Marty to Jennifer and back, the same FormControl instance, which got manipulated in the meantime, is passed to the input component.

To circumvent this issue, we want to pass a new instance every time the control property changes:

...
render: ({ control }: {control: any}) => {
    const controlKey = control as keyof typeof mapping
    return ({
      props: {
        control: mapping[controlKey]?.(),
      },
    })
  },

So the control parameter equals Marty, Jennifer or Doc, and as soon as the control is changing, the render function gets called. Here you are able to map your properties without any limitations. As I said, you want to pass a fresh FormControl instance to the input component.

Let's take a look at the mapping object:

const mapping = {
  Marty: () =>
    new FormControl({ value: 'Marty McFly', disabled: false }, [
      Validators.required,
    ]),
  Jennifer: () =>
    new FormControl({ value: 'Jennifer Parker', disabled: true }, [
      Validators.required,
    ]),
  Doc: () =>
    new FormControl({ value: 'Doc Brown', disabled: false }, [
      Validators.required,
    ]),
};

This object maps the control string values to callback functions, which return a fresh FormControl instance, every time the mapping key is called. So calling mapping['Marty'] creates a new FormControl instance. So in principle mapping[value] are factory functions, which return instances of FormControl. Now hopefully, this line makes sense:

control: mapping[controlKey]?.(),

If controlKey is e.g. Marty, then mapping['Marty']?.() returns a fresh instance of FormControl.

I hope all of this makes sense and the workaround works sufficiently for you.

valentinpalkovic commented 1 year ago

Because the render function workaround solves also the FormControl use case, I am closing this issue. Please feel free to reopen it, if the problem still occurs.

work933k commented 1 year ago

I tried this solution, but i think the issue does occur when trying to provision a FormGroup into a Story.. The only solution i found to work is given here: delete form['controls']['test']['_parent']

valentinpalkovic commented 1 year ago

@work933k could you provide a reproduction?

work933k commented 1 year ago
const formGroup = new FormGroup({
  ondertekend: new FormControl(null, Validators.required),
});
delete formGroup['controls']['ondertekend']['_parent'];  <=  Without this line, the error would occur

export const Primary: StoryObj<StepSummaryUiComponent> = {
  render: (args: StepSummaryUiComponent) => ({
    props: {
      ...args,
      formGroup: formGroup,
    },
  }),
  args: {
    querystringParameters: { isExternePartner: true, stylesheet: '' },
    state: state,
  },
};

@storybook/angular@7.0.9

Errors on JSON.stringify:

exports.computesTemplateFromComponent = computesTemplateFromComponent;
const createAngularInputProperty = ({ propertyName, value, argType, }) => {
    let templateValue;
    switch (typeof value) {
        case 'string':
            templateValue = `'${value}'`;
            break;
        case 'object':
            templateValue = JSON.stringify(value)
                .replace(/'/g, '\u2019')
                .replace(/\\"/g, '\u201D')
                .replace(/"([^-"]+)":/g, '$1: ')
                .replace(/"/g, "'")
                .replace(/\u2019/g, "\\'")
                .replace(/\u201D/g, "\\'")
                .split(',')
                .join(', ');
            break;
        default:
            templateValue = value;
    }
    return `[${propertyName}]="${templateValue}"`;
};
@Component({
  selector: 'dummy-step-summary-ui',
  templateUrl: './ui.component.html',
  styleUrls: ['./ui.component.scss'],
})
export class StepSummaryUiComponent {
  @Input() formGroup!: UntypedFormGroup;
  @Input() querystringParameters: QuerystringParameters;
  @Input() distributionCosts = 0;
  @Input() closingCosts = 0;
  @Input() perEmployeeCosts = 0;
  @Input() privacyStatementUrl = '';
  @Input() disclaimerUrl = '';
  @Input() cookieStatementUrl = '';
  @Input() webUrl = '';
work933k commented 1 year ago

@valentinpalkovic Why did this issue get closed?

valentinpalkovic commented 1 year ago

Couldn’t you use the same mapping workaround with the FormGroup as well, like shown with the FormControl?

work933k commented 1 year ago

I tried, but i wasn't able to get it to work. I'll try it again.

work933k commented 1 year ago

@valentinpalkovic I tried the mapping-example again, but that is not the solution nor the problem. The issue is having a FormControl in the FormGroup. The FormControl apparently has a property which loops back to the FormGroup. You'll either see errors in the browser-console or after switching the mapping the "JSON serialization" error message.

I modified the provided Stackblitz: https://stackblitz.com/edit/github-3znyoe-6cjwh3?file=src/stories/input.component.stories.ts

In the browser-console you should see errors about the formgroup.

valentinpalkovic commented 1 year ago

@work933k Thank you for the reproduction. Indeed, I could reproduce it for FormGroup. Do you think, it makes sense to reopen this issue or open a new one? Does it still relate to the original issue?

work933k commented 1 year ago

@valentinpalkovic I'm not too sure if the FormGroup issue is exactly the same as with MatTable. Maybe good to continue on this thread which is related about FormGroup: https://github.com/storybookjs/storybook/discussions/15602#discussioncomment-5730841

OanaSurdea commented 1 year ago

@shilman it helped, thank you, should I close the issue or any fix is expected (e.g. provide a message in addon about required mapping instead of story crash?)

It is not clear what the complete workaround/fix for this is. Neither one of the workarounds mentioned worked for me: Workaround 1:

dataSource: {
  table: {
    disable: true,
  }
}

Workaround 2:

const dataSource = new MatTableDataSource(ELEMENT_DATA);

export default {
  title: 'foo',
  argTypes: {
    testDataSource: {
      mapping: { Default: dataSource }
    }
  }
}
export const Primary = Template.bind({});
Primary.args = {
  testDataSource: 'Default'
}

I've tested using the code from your https://github.com/Tatsianacs/storybook--bug demo, Angular & Material v14, Storybook v7.

Here is a Stackblitz link: https://stackblitz.com/edit/angular-material-storybook.

Could you or anyone please share more details about how you solved the issue? Thank you 🙏

maidmehic commented 1 year ago

Does this help: https://github.com/storybookjs/storybook/discussions/15602#discussioncomment-6290582?

Cloud9Clone commented 1 year ago

After having a chat internally, we figured out a workaround: https://stackblitz.com/edit/github-3znyoe-aebjsf?file=src%2Fstories%2Finput.component.stories.ts

So instead of using the native mapping functionality of Storybook, you can use the render property to do the mapping manually:

...
const mapping = {...};

const meta: Meta<InputField> = {
  ...
  argTypes: {
    control: {
      options: ['Marty', 'Jennifer', 'Doc'],
      control: {
        type: 'select',
      },
    },
  },
  render: ({ control }: {control: any}) => {
    const controlKey = control as keyof typeof mapping
    return ({
      props: {
        control: mapping[controlKey]?.(),
      },
    })
  },
};
...

The main issue is that the FormControl instances get only instantiated once. Even if you switch the control values in the ArgsTable for control, the same FormControl instance is used, which was instantiated initially. A new instance is NOT created. That's a limitation of the current implementation of Storybook's mapping capabilities. It seems that as soon as an input element gets rendered, the FormControl instance changes internally and creates a self-reference. If you now switch from Marty to Jennifer and back, the same FormControl instance, which got manipulated in the meantime, is passed to the input component.

To circumvent this issue, we want to pass a new instance every time the control property changes:

...
render: ({ control }: {control: any}) => {
    const controlKey = control as keyof typeof mapping
    return ({
      props: {
        control: mapping[controlKey]?.(),
      },
    })
  },

So the control parameter equals Marty, Jennifer or Doc, and as soon as the control is changing, the render function gets called. Here you are able to map your properties without any limitations. As I said, you want to pass a fresh FormControl instance to the input component.

Let's take a look at the mapping object:

const mapping = {
  Marty: () =>
    new FormControl({ value: 'Marty McFly', disabled: false }, [
      Validators.required,
    ]),
  Jennifer: () =>
    new FormControl({ value: 'Jennifer Parker', disabled: true }, [
      Validators.required,
    ]),
  Doc: () =>
    new FormControl({ value: 'Doc Brown', disabled: false }, [
      Validators.required,
    ]),
};

This object maps the control string values to callback functions, which return a fresh FormControl instance, every time the mapping key is called. So calling mapping['Marty'] creates a new FormControl instance. So in principle mapping[value] are factory functions, which return instances of FormControl. Now hopefully, this line makes sense:

control: mapping[controlKey]?.(),

If controlKey is e.g. Marty, then mapping['Marty']?.() returns a fresh instance of FormControl.

I hope all of this makes sense and the workaround works sufficiently for you.

Is this solution compatible with Storybook version 6.5?

sebastienservouze commented 10 months ago

Hi ! I had this annoying issue as well with my Form stories...

But I found a trick which maintains a fully fonctionnal FormGroup (with valueChanges working properly) in stories 🎉

Storybook tries to stringify any args you pass to a component. We can just tell our formGroups to be stringified by our rules. By setting toJSON() on our objects, we can prevent Storybook from trying to serialize a circular structure.

const MY_FORM = new FormGroup({
    a: new FormControl('A'),
    b: new FormControl('B'),
})

// Tell JSON.stringify to serialize it as null > No circular dependency
// You could return any value you like though
MY_FORM['toJSON'] = () => null; 

export const FormStory: Story = {
    name: 'FormStory',
    args: {
        form: MY_FORM
    },
};

Note that this would work for ANY object that circles when stringified