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

@storybook/source-loader messes with custom hooks - evaluates source code? #8294

Closed
donaldpipowitch opened this issue Oct 4, 2019 · 7 comments

Comments

@donaldpipowitch
Copy link
Contributor

donaldpipowitch commented Oct 4, 2019

Describe the bug
I have a custom component using a custom hook. The hook runs an assertion on the context at runtime. I don't know why, but it looks like the source loader runs parts of its logic on my context/hook which throws an error.

To Reproduce
Clone this repo and follow README:
https://github.com/donaldpipowitch/storybook-bug-demo-source-loader-context-null

Expected behavior
No error should happen.

Screenshots
image

image

null is a valid value in my context.

System:

Environment Info:

  System:
    OS: macOS Mojave 10.14.6
    CPU: (16) x64 Intel(R) Core(TM) i9-9980HK CPU @ 2.40GHz
  Binaries:
    Node: 10.16.0 - ~/.nvm/versions/node/v10.16.0/bin/node
    Yarn: 1.17.3 - ~/.yarn/bin/yarn
    npm: 6.9.0 - ~/.nvm/versions/node/v10.16.0/bin/npm
  Browsers:
    Chrome: 77.0.3865.90
    Safari: 13.0.1
  npmPackages:
    @storybook/addon-a11y: ^5.2.1 => 5.2.1 
    @storybook/addon-backgrounds: ^5.2.1 => 5.2.1 
    @storybook/addon-docs: ^5.2.1 => 5.2.1 
    @storybook/react: ^5.2.1 => 5.2.1 
    @storybook/source-loader: ^5.2.1 => 5.2.1 
    @storybook/theming: ^5.2.1 => 5.2.1 
@celinrayen
Copy link

Can I take a look on it if it the issue is still open?

@donaldpipowitch
Copy link
Contributor Author

The issue should still be open. 🙂 Thank you.

@stale
Copy link

stale bot commented Oct 26, 2019

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!

@stale stale bot added the inactive label Oct 26, 2019
@stale stale bot removed the inactive label Oct 26, 2019
@donaldpipowitch
Copy link
Contributor Author

donaldpipowitch commented Nov 5, 2019

I saw there were some changes to the source-loader (#8499), but this issue still seems to be preset.

  npmPackages:
    @storybook/addon-a11y: ^5.3.0-alpha.38 => 5.3.0-alpha.38 
    @storybook/addon-backgrounds: ^5.3.0-alpha.38 => 5.3.0-alpha.38 
    @storybook/addon-docs: ^5.3.0-alpha.38 => 5.3.0-alpha.38 
    @storybook/react: ^5.3.0-alpha.38 => 5.3.0-alpha.38 
    @storybook/source-loader: ^5.3.0-alpha.38 => 5.3.0-alpha.38 
    @storybook/theming: ^5.3.0-alpha.38 => 5.3.0-alpha.38 

Updated Test Repo: https://github.com/donaldpipowitch/storybook-bug-demo-source-loader-context-null

@shilman
Copy link
Member

shilman commented Nov 11, 2019

Yo-ho-ho!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.3.0-alpha.43 containing PR #8773 that references this issue. Upgrade today to try it out!

You can find this prerelease on the @next NPM tag.

Closing this issue. Please re-open if you think there's still more to do.

@shilman shilman closed this as completed Nov 11, 2019
@donaldpipowitch
Copy link
Contributor Author

Thank you for the quick release 👏

@shilman
Copy link
Member

shilman commented Dec 2, 2019

Whoopee!! I just released https://github.com/storybookjs/storybook/releases/tag/v5.2.8 containing PR #8773 that references this issue. Upgrade today to try it out!

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