Controls: Properties with controls use name of the assigned variable as their value

[pvds]

The bug

Since storybook 6.4 properties also render a control.

When assigning an enum to a property the defaultValue is interpreted as a string instead of an enum.
This breaks my components since it receives the name of the enum as a string instead of the enum itself.

After some further investigation the same behaviour occurs with other data types.
When assigning a variable to a property the default value takes the name of the variable instead of it's value(s).

Only when directly assigning a value:

• number/boolean render correct control using the value (as expected)
• array/object render as a string containing the value (not as an Object control)

I have updated the repro repo and the deployed version to add these cases.

To Reproduce

Storybook 6.4.3

With properties using controls the default value becomes a string containing the name of the assigned variable

• Repro repository
  https://github.com/pvds/storybook-repro-controls-property-bug
• Deployed enum bug story on chromatic
  https://61a7c73e0e1dc3004a668f13-medaxqhpau.chromatic.com/?path=/story/reproduction-property-control-bug--basic

Storybook 6.3.12

Without properties using controls it works as expected

• Repro repository
  https://github.com/pvds/storybook-repro-controls-property-bug/tree/test/storybook-6.3.12
• Deployed enum bug story on chromatic
  https://61a7c73e0e1dc3004a668f13-kxfauvlnox.chromatic.com/?path=/story/reproduction-property-control-bug--basic

System

Environment Info:

  System:
    OS: Windows 10 10.0.19044
    CPU: (8) x64 Intel(R) Core(TM) i7-10510U CPU @ 1.80GHz
  Binaries:
    Node: 14.16.0 - C:\Program Files\nodejs\node.EXE
    Yarn: 3.1.1 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 6.14.11 - C:\Program Files\nodejs\npm.CMD
  Browsers:
    Edge: Spartan (44.19041.1266.0), Chromium (96.0.1054.34)
  npmPackages:
    @storybook/addon-actions: ^6.4.3 => 6.4.3
    @storybook/addon-docs: ^6.4.3 => 6.4.3
    @storybook/addon-essentials: ^6.4.3 => 6.4.3
    @storybook/addon-links: ^6.4.3 => 6.4.3
    @storybook/angular: ^6.4.3 => 6.4.3
    @storybook/builder-webpack5: ^6.4.3 => 6.4.3
    @storybook/manager-webpack5: ^6.4.3 => 6.4.3

Additional context

I'm using Angular with compodoc and I know there are limitations due to the way compodoc works.
Rendering controls for properties does however often break stories due to the defaultValue string compodoc provides.
It's a common use case to assign an imported variable to a property in order to use it in your template.

The only way to work around the new behaviour is to manually exclude or disable all the properties through controls.
In our enterprise monorepo this would result in a huge number of excludes or disabled controls, resulting in a lot of overhead.
.

Relevant Compodoc data

The compodoc data generated for the myEnumProperty property and the myEnum enum used in the reproduction.

propertiesClass

{
    "name": "myEnumProperty",
    defaultValue": "myEnum",
    deprecated": false,
    deprecationMessage": "",
    type": "",
    optional": false,
    description": "<p>Enum assigned to property</p>\n<p>Storybook controls renders the enum name &quot;myEnum&quot; as it&#39;s value using string data type\nIt seems to be interpreting the defaultValue from compodoc as a string</p>\n",
    line": 33,
    rawdescription": "\n\nEnum assigned to property\n\nStorybook controls renders the enum name \"myEnum\" as it's value using string data type\nIt seems to be interpreting the defaultValue from compodoc as a string\n"
}

enumerations

{
    name": "myEnum",
    childs": [
        {
            name": "foo",
            deprecated": false,
            deprecationMessage": "",
            value": "foo value"
        },
        {
            name": "bar",
            deprecated": false,
            deprecationMessage": "",
            value": "bar value"
        }
     ],
     ctype": "miscellaneous",
     subtype": "enum",
     deprecated": false,
     deprecationMessage": "",
     description": "",
     file": "src/stories/enum-bug.component.ts"
}

Expected solution

I would expect the actual values of the variables assigned to the property to render with the correct control.
There might be compodoc limitations blocking this.

As an alternative solution (a global option for) not rendering controls for properties would be acceptable.

[astik]

Here is a complementary case, for a component implementing ControlValueAccessor, we have class properties for onChange and onTouch:

  onChange: any = () => {};
  onTouch: any = () => {};

Clicking on the component will result in error ERROR TypeError: this.onChange is not a function as onClick is interpreted as string in the story.

Disabling the control does not help:

  parameters: {
    controls: {
      disable: true
    },
  },

We need to exclude explicitly wrongly-interpreted properties:

  parameters: {
    controls: {
      disable: true,
      exclude: ['onChange', 'onTouch'],
    },
  },

Same goes with using QueryList properties:

  @ContentChildren(RadioButtonComponent)
  buttons: QueryList<RadioButtonComponent> = new QueryList<RadioButtonComponent>();

which will results in error:

ERROR Error: Error trying to diff 'new QueryList<RadioButtonComponent>()'. Only arrays and iterables are allowed
// or
ERROR TypeError: this.buttons.forEach is not a function

(FWIW, it was tested with Compodoc 1.1.15 and 1.1.16 and Storybook 6.4.5)

[pvds]

@astik indeed having to exclude them everywhere (especially in a large monorepo) is tedious, bloats stories and removes the properties/methods from the documentation.

Inaddition I can also see problems arising with methods since these also receive controls since Storybook 6.4.

[astik]

I had the problem in the 6.3 branch already, it was fixed (also in the 6.3 branch) by a combination of upgrading Storybook and / or Compodoc ... still i can't remember when it occurs precisely.
I have no doubt a clean solution will be found =)

[pvds]

@astik in my case it seems strictly related to 6.4 changes (potentialy in combination with how compodoc documentation interacts with it)

@shilman thanks for picking this up!

[shilman]

@pvds thanks for the clean repro! (haven't actually run it yet, but super useful .. shouldn't be hard to fix, hopefully in the next day or two)

[pvds]

@pvds thanks for the clean repro! (haven't actually run it yet, but super useful .. shouldn't be hard to fix, hopefully in the next day or two)

@shilman you're welcome, I tried to keep it as clean as possible.
It was a nice excuse to play around with Chromatic!

Thanks in advance for the effort.

[pvds]

I've tried to debug the issue in your code base on my local machine but it's quite a deep dive and I'm not familiarized enough with the storybook repo.
So unfortunately no luck from my side.

@shilman do you have a status update?

[fdabrowski]

@shilman, any updates about that issue? Thanks!

[shilman]

Will try to look at it today @fdabrowski @pvds

[pvds]

@rothsandro as you've mentioned #17004 and #16959 are also related issues.

Maybe good to note is that from what I have tested the issues started since Storybook 6.4 which renders controls for properties and methods too. Without the controls Storybook doesn't 'hijack' the values.

[stale]

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!

[jfgreffier]

According to #17004 it seem linked with the docs addon

[pvds]

@shilman do you have an update on when you plan to fix this?

[fikkatra]

Also waiting for a fix, as the majority of the component stories in our design system are now broken. Experiencing issues with enum properties, as well as errors this. is not a function, as @astik mentions.

[amaino]

Seems to be related to compodoc because if I run compodoc with these flags --disablePrivate --disableInternal the private and internal flagged properties does not become strings.

[jogelin]

Any updates on this?

I am in the process of Nrw/Nx/Angular 13 migration and it is using storybook ~6.4.0 but this one is blocking my progress :p

I am already using the --disablePrivate --disableInternal but still having issues for @Input that have default inferred values. However in storybook, even if the value is a function, the string of compodoc is used

[pvds]

@shilman as @amaino states in #17362 (comment) the issue is caused by the removal of let value = eval(property.defaultValue); in commit 9f24a79 which went out in v6.4.0-alpha.34.

---

Using storybook 6.4.18 I changed the compiled compodoc.js file to use var value = eval(property.defaultValue); and it resolves the regression.

I know eval is considered evil but seen the regression it has caused I would propose to either revert this change or come up with a working alternative not resulting in the blocking regression it has caused now.

The PR (#17353) by @amaino basically replaces eval with new Function which would be a minor improvement compared to eval since using new Function the executed code cannot access local variables because the code runs in a separate scope. I'm not sure whether his PR resolves the regression though.

---

@shilman as the root cause seems clear now could you look into resolving this regression?

PS. considering the user experience I do not see any benefit in rendering controls for Outputs, Properties or Methods in Angular Storybook since they are not meant to be 'controlled'.

[shilman]

@pvds Yes my understanding is that the proper fix is to remove the controls altogether for those values, not to re-introduce the eval.

[amaino]

@pvds Yes my understanding is that the proper fix is to remove the controls altogether for those values, not to re-introduce the eval.

If I understand you correctly you mean remove controls for Output, Properties snd Methods and that I agree with mostly as there could be scenarios were you would want to access public properties that are not inputs. But you would still need to use eval for inputs to get corect default values on the controls at least with how it is implemented today or am I missing something?

[pvds]

@amaino you are correct that only removing controls for for Outputs, Properties and Methods does not completely fix the regression introduced by removing the eval. The change also affects Inputs.

When I have some time i'll update my reproduction examples with @input() properties to show the regression.

---

As Storybook 6.3 does not work with Angular 13 esm modules I cannot wait anymore with upgrading to Storybook 6.4 so I've patched the @storybook/addon-docs package to use eval again. I'm using https://www.npmjs.com/package/patch-package for this with the attached patch.

diff --git a/node_modules/@storybook/addon-docs/dist/esm/frameworks/angular/compodoc.js b/node_modules/@storybook/addon-docs/dist/esm/frameworks/angular/compodoc.js
index 25e13af..dc6c42e 100644
--- a/node_modules/@storybook/addon-docs/dist/esm/frameworks/angular/compodoc.js
+++ b/node_modules/@storybook/addon-docs/dist/esm/frameworks/angular/compodoc.js
@@ -260,9 +260,7 @@ var extractDefaultValueFromComments = function extractDefaultValueFromComments(p
 var extractDefaultValue = function extractDefaultValue(property) {
   try {
     var _property$defaultValu, _property$jsdoctags;
-
-    var value = (_property$defaultValu = property.defaultValue) === null || _property$defaultValu === void 0 ? void 0 : _property$defaultValu.replace(/^'(.*)'$/, '$1');
-    value = castDefaultValue(property, value);
+    var value = eval(property.defaultValue);
 
     if (value == null && ((_property$jsdoctags = property.jsdoctags) === null || _property$jsdoctags === void 0 ? void 0 : _property$jsdoctags.length) > 0) {
       value = extractDefaultValueFromComments(property, value);

NB this is a rudimentary patch which does not remove controls for Outputs, Properties and Methods but at least doesn't initialize them so it doesn't hijack the values

[akhileshThapliyal]

May I know when will this issue be fixed, as it is impacting my storybook components? The existing components of my project have several attributes. I agree with the above suggestion, there is no need of displaying Outputs, Properties and Methods in the controls addon.

[pvds]

@pvds Yes my understanding is that the proper fix is to remove the controls altogether for those values, not to re-introduce the eval.

@shilman has removing the controls for everything except Input properties been planned to be implemented,
or is this still an internal discussion?

[JamesHenry]

Related to this point around only considering inputs, the only way we found to unblock ourselves on the latest storybook + Angular on a particular client project was to apply the following patch in the short term:

@storybook+addon-docs+6.4.19.patch

diff --git a/node_modules/@storybook/addon-docs/dist/esm/frameworks/angular/compodoc.js b/node_modules/@storybook/addon-docs/dist/esm/frameworks/angular/compodoc.js
index 25e13af..a09d519 100644
--- a/node_modules/@storybook/addon-docs/dist/esm/frameworks/angular/compodoc.js
+++ b/node_modules/@storybook/addon-docs/dist/esm/frameworks/angular/compodoc.js
@@ -287,7 +287,7 @@ var resolveTypealias = function resolveTypealias(compodocType) {
 
 export var extractArgTypesFromData = function extractArgTypesFromData(componentData) {
   var sectionToItems = {};
-  var compodocClasses = ['component', 'directive'].includes(componentData.type) ? ['propertiesClass', 'methodsClass', 'inputsClass', 'outputsClass'] : ['properties', 'methods'];
+  var compodocClasses = ['component', 'directive'].includes(componentData.type) ? ['inputsClass'] : ['properties', 'methods'];
   compodocClasses.forEach(function (key) {
     var data = componentData[key] || [];
     data.forEach(function (item) {

[salmoro]

Only reasonable solution which worked for me was to remove the "component" property of the story definition and to make sure the component is included in the declarations (or in a the declarations of an imported module) as mentioned by someone on a related issue.

[mrlonis]

I can confirm removing the component property of the story definition also works for me without the need for disabling the docs addon

[jfgreffier]

Only reasonable solution which worked for me was to remove the "component" property of the story definition and to make sure the component is included in the declarations (or in a the declarations of an imported module) as mentioned by someone on a related issue.

@salmoro Who ?
What related issue ?

I'll go with the patch and search for alternatives to Storybook

[swiner-dlpx]

Just want to chime in that I also see this affecting Stories in my projects. In particular this breaks RxJS Observable pipelines.

[EliezerB123]

Just want to chime in that I also see this affecting Stories in my projects. In particular this breaks RxJS Observable pipelines.

@swiner-dlpx Oh jeez. THANK YOU!

THAT'S why my component is getting errors. (I'm in middle of migrating from v5 Knobs to v6 Controls, and I couldn't for the life of me figure out what I was doing wrong.... Turns out that like you said, it's getting confused because I have a string attached to my @Output() EventEmitter(). )

My exact error was:

 ERROR TypeError: ctx_r2.myOutput is not a function
    at StorybookWrapperComponent_div_9_Template_myComponent_onMyOutput_1_listener

And, in case anyone else comes here from google, this fixed it:

// myComponent.story.ts
export default {
  parameters: {
    controls: {
      exclude: ['myOutputOne', 'myOutputTwo','myOutputThree'],
    },
  },
} as Meta;

[shilman]

Great Caesar's ghost!! I just released https://github.com/storybookjs/storybook/releases/tag/v7.0.0-alpha.57 containing PR #19935 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.

[EliezerB123]

@shilman Will this get backported into v6.x? v7 is still in alpha.....

[shilman]

@EliezerB123 mind testing the beta? If folks here can confirm that the fix works, I'll investigate porting it back to 6.5

[sonikasharma1403]

Any update on how to do this???