storybookjs / storybook

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

[Bug]: spyOn on Angular's services methods stopped working after upgrade to Stroybook 8 #28419

Closed dmy-leanix closed 1 day ago

dmy-leanix commented 3 days ago

Describe the bug

Currently I'm trying to upgrade to Storybook 8 my Angular's app storybook with interaction tests. In some cases I have in play function the following - getting injector, finding needed service and spying on some methods:

const injector = args.getInjector();
const service = injector.get(MyService);
const spy = spyOn(service, 'test');

// ... some UI actions which should call that service's method

expect(spy).toHaveBeenCalled();

What's wrong, service's method losts his scope to null, so any this usage will cause errors inside that method

In Storybook 7 it was working correctly

Reproduction link

https://stackblitz.com/edit/github-ex87bm

Reproduction steps

No response

System

System:
    OS: macOS 14.4.1
    CPU: (8) arm64 Apple M1 Pro
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 18.17.1 - ~/.nvm/versions/node/v18.17.1/bin/node
    npm: 9.6.7 - ~/.nvm/versions/node/v18.17.1/bin/npm <----- active
  Browsers:
    Chrome: 126.0.6478.127
    Safari: 17.4.1
  npmPackages:
    @storybook/addon-a11y: 8.1.7 => 8.1.7 
    @storybook/addon-actions: 8.1.7 => 8.1.7 
    @storybook/addon-backgrounds: 8.1.7 => 8.1.7 
    @storybook/addon-controls: 8.1.7 => 8.1.7 
    @storybook/addon-docs: 8.1.7 => 8.1.7 
    @storybook/addon-essentials: 8.1.7 => 8.1.7 
    @storybook/addon-interactions: 8.1.7 => 8.1.7 
    @storybook/addon-knobs: ^8.0.1 => 8.0.1 
    @storybook/addon-mdx-gfm: 8.1.7 => 8.1.7 
    @storybook/addon-measure: 8.1.7 => 8.1.7 
    @storybook/addon-outline: 8.1.7 => 8.1.7 
    @storybook/addon-toolbars: 8.1.7 => 8.1.7 
    @storybook/addon-viewport: 8.1.7 => 8.1.7 
    @storybook/angular: 8.1.7 => 8.1.7 
    @storybook/blocks: 8.1.7 => 8.1.7 
    @storybook/core-events: 8.1.7 => 8.1.7 
    @storybook/core-server: 8.1.7 => 8.1.7 
    @storybook/preview-api: 8.1.7 => 8.1.7 
    @storybook/test: 8.1.7 => 8.1.7 
    @storybook/test-runner: ^0.19.0 => 0.19.0 
    @storybook/types: 8.1.7 => 8.1.7 
    eslint-plugin-storybook: ^0.8.0 => 0.8.0

Additional context

No response

dmy-leanix commented 3 days ago

Can it be related to this line in spy.ts?

const originalMockImplementation = reactive.mockImplementation.bind(null);
bawjensen commented 3 days ago

yeah, it's either that line in fn() or this line in listenWhenCalled(): https://github.com/storybookjs/storybook/blob/29b9db7cde7cfa894b17f1b8bdebe3155c4dc4df/code/lib/test/src/spy.ts#L49

We've been suffering from the latter (found through debugging/looking at the stack trace), which was introduced here: https://github.com/storybookjs/storybook/commit/6ea5bd899fbc0c82699637aeb4eac20a61de9e73#diff-7e286e50b9508a96cb4512909ee1c653b07ebd3dc930d381b7b06115975d4effR44 (same commit you linked, just a few lines lower)

I believe @kasperpeulen confirmed this appears to be a bug, and it ought to be binding an appropriate this