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

storyStoreV7 Angular 12 causes issues with ngOnDestroy and subscriptions #16959

Closed
fehrm opened this issue Dec 9, 2021 · 11 comments
Closed

storyStoreV7 Angular 12 causes issues with ngOnDestroy and subscriptions #16959

fehrm opened this issue Dec 9, 2021 · 11 comments

Comments

@fehrm
Copy link

fehrm commented Dec 9, 2021

Describe the bug
in a Angular component using ngOnDestroy to unsubscribe from a RXJS Subscription when navigating away you get "this.subscriptions.unsubscribe is not a function"

To Reproduce

  1. Create a Angular component utilizing a service that uses for example http pipe.
  2. Add a variable for the subscription, private subscription = new Subscription();
  3. Implement onDestroy with this.subscription.unsubscribe();
  4. Create simple CSF3 story for this component and service
  5. Navigate to the component in Storybook
  6. Navigate away
  7. Error occurs, "this.subscriptions.unsubscribe is not a function"

What I think happens is that the variable subscription gets exposed in Storybook, and then initialized as a string, so when you put a breakpoint in the ngOnDestroy, this.subscription is, after navigating away from the story in Storybook, an actual string with the value "new Subscription", instead of it being an actual RXJS Subscription object. This of course makes it hard to call .unsubscribe() on the string.

Reproduction repo

System
System:
OS: Windows 10 10.0.19043
CPU: (16) x64 AMD Ryzen 7 2700X Eight-Core Processor
Binaries:
Node: 16.7.0 - c:\program files\nodejs\node.EXE
Yarn: 3.1.1 - c:\program files\nodejs\yarn.CMD
npm: 7.21.0 - c:\program files\nodejs\npm.CMD
Browsers:
Chrome: 86.0.4240.183
Edge: Spartan (44.19041.1266.0), Chromium (96.0.1054.43)
npmPackages:
@storybook/addon-actions: ^6.5.0-alpha.2 => 6.5.0-alpha.2
@storybook/addon-docs: ^6.5.0-alpha.2 => 6.5.0-alpha.2
@storybook/addon-essentials: ^6.5.0-alpha.2 => 6.5.0-alpha.2
@storybook/addon-links: ^6.5.0-alpha.2 => 6.5.0-alpha.2
@storybook/angular: ^6.5.0-alpha.2 => 6.5.0-alpha.2
@storybook/builder-webpack5: ^6.5.0-alpha.2 => 6.5.0-alpha.2
@storybook/manager-webpack5: ^6.5.0-alpha.2 => 6.5.0-alpha.2

Additional context
I get it more often when using storyStoreV7: true in my main project, but can reproduce it without it. This wasn't an issue in previous version of Storybook.
Don't think it matters that it is a CSF3 story.

I found the error on 6.4.8, but used @next as you guided in this template to create repro repo.

@rothsandro
Copy link

All props are converted to strings when using controls, see also #17001

@joewIST
Copy link

joewIST commented Dec 22, 2021

I am also having this issue- anything called in the ngOnDestroy isn't being evaluated as an object causing an error. It can be resolved by refreshing but obviously this can be tedious.

Any workaround for this before a fix?

@rothsandro
Copy link

As a workaround we now use include to specify which properties should have controls. All other props are then hidden in the controls section (but still visible in the args table). This needs to be done for every story and it's not type-safe.

parameters: {
  controls: {
    include: ['prop1', 'prop2'],
  },
},

@joewIST
Copy link

joewIST commented Dec 22, 2021

This doesn't solve the issue re: ngOnDestroy, and also results in another error "Cannot read properties of undefined (reading 'action')". We also have a very large library and so going through and updating each stories.ts file is tedious. I think we will just have to live with refreshing the browser every time we navigate away from a story until a fix is in place.

@sthuber90
Copy link

I have come across the same error on 6.4.19

// package.json
{
   "devDependencies":{
      "@storybook/addon-a11y":"^6.4.19",
      "@storybook/addon-actions":"^6.4.19",
      "@storybook/addon-docs":"^6.4.19",
      "@storybook/addon-essentials":"^6.4.19",
      "@storybook/addon-links":"^6.4.19",
      "@storybook/angular":"^6.4.19",
      "@storybook/builder-webpack5":"^6.4.19",
      "@storybook/manager-webpack5":"^6.4.19"
   }
}

I have adjusted my angular.json like the following to resolve the problem from #16256

{
  "storybook": {
    "projectType": "library",
    "root": "./",
    "architect": {
      "start": {
        "builder": "@storybook/angular:start-storybook",
        "options": {
          "tsConfig": ".storybook/tsconfig.json",
          "styles": [
            ".storybook/assets/_commons.scss"
          ],
          "port": 6006,
          "open": true
        }
      },
      "build": {
        "builder": "@storybook/angular:build-storybook",
        "options": {
          "tsConfig": ".storybook/tsconfig.json",
          "styles": [
            ".storybook/assets/_commons.scss"
          ],
          "port": 6006
        }
      }
    }
  }
}

To get ngOnDestroy to work, I've changed my preview.js:

export const parameters = {
    controls: {
        exclude: ['unsubscribe$'],
    }
}

... and main.js:

module.exports = {
    core: {
        builder: "webpack5",
    },
    "stories": [
        "../src/**/*.stories.mdx",
        "../src/**/*.stories.@(js|jsx|ts|tsx)"
    ],
    "addons": [
        {
            name: '@storybook/addon-essentials',
            options: {
                actions: false,
            }
        }
    ],
}

I know it's still only a workaround and I cannot use the actions add-on anymore with the current state but atleast the stories work correctly

@freon27
Copy link

freon27 commented Mar 22, 2022

Following this thread, I tried out setting angularLegacyRendering to true in preview.js and it resolved my onDestroy errors.

Obviously it's only a temporary workaround but it has unblocked upgrading to ng13 for me.

@joewIST
Copy link

joewIST commented Mar 22, 2022

this didn't work for me, and resulted in the following error:

This constructor is not compatible with Angular Dependency Injection because its dependency at index 0 of the parameter list is invalid.
This can happen if the dependency type is a primitive like a string or if an ancestor of this class is missing an Angular decorator.

Please check that 1) the type for the parameter at index 0 is correct and 2) the correct Angular decorators are defined for this class and its ancestors. ; Zone: ; Task: Promise.then ; Value: Error: This constructor is not compatible with Angular Dependency Injection because its dependency at index 0 of the parameter list is invalid.
This can happen if the dependency type is a primitive like a string or if an ancestor of this class is missing an Angular decorator.

@freon27
Copy link

freon27 commented Mar 22, 2022

What are the constructor parameters for the class it's complaining about?

@joewIST
Copy link

joewIST commented Mar 22, 2022

it happens on every component

@joewIST
Copy link

joewIST commented Mar 22, 2022

my workaround is: 1) remove the component property and 2) add the component to the declarations in the decorators object:

title: 'chart/shared-ui-line-chart',
// component: LineChartComponent,
decorators: [
    moduleMetadata({
        declarations: [LineChartComponent],
        imports: [BaseButtonModule],
        providers: [DecimalPipe]
    })
],

this works for me (for now) on sb 6.4.19

@shilman
Copy link
Member

shilman commented Dec 3, 2022

Yay!! 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.

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

6 participants