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

Verify that name is a string in addons/actions #1415

Merged
merged 1 commit into from
Jul 7, 2017

Conversation

thomasthesecond
Copy link
Contributor

Issue: Within addon/actions, it appears that it’s possible that name can be passed in as an object when a string is expected. This causes the error seen below in Chrome (version 59.0.3071.115):

image

I’ve noticed that this error doesn’t always happen. If Storybook loads with a component that uses the action addon, there is no error. However, if you navigate to a component that uses the action addon, then the error appears.

What I did

This pull request adds a condition to the fnName constant that verifies the value of name is a string.

How to test

  • Create a component that uses the action addon
  • Launch Storybook and navigate to the component with the action from the left-hand menu
  • JavaScript error message should appear

As I mentioned above, the error only seems to happen when you navigate to a component by clicking on its name from the menu.

I wanted to write a test for this, but wasn’t quite sure if the fnName constant needs to be extracted from the action function in order to make it testable.

It appears that it’s possible that `name` can be passed in
as an object. In order for the `replace` method to work,
name must be a string. This pull request adds a condition to
the `fnName` constant that verifies the value of `name` is
a string.
@codecov
Copy link

codecov bot commented Jul 5, 2017

Codecov Report

Merging #1415 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1415   +/-   ##
=======================================
  Coverage   14.35%   14.35%           
=======================================
  Files         201      201           
  Lines        4612     4612           
  Branches      505      494   -11     
=======================================
  Hits          662      662           
- Misses       3499     3526   +27     
+ Partials      451      424   -27
Impacted Files Coverage Δ
addons/actions/src/preview.js 50% <100%> (ø) ⬆️
app/react/src/server/iframe.html.js 29.26% <0%> (ø) ⬆️
lib/ui/src/libs/key_events.js 23.25% <0%> (ø) ⬆️
addons/knobs/src/components/WrapStory.js 12% <0%> (ø) ⬆️
lib/ui/src/modules/ui/configs/handle_routing.js 28.94% <0%> (ø) ⬆️
app/react-native/src/bin/storybook-build.js 0% <0%> (ø) ⬆️
app/react/src/server/babel_config.js 44.82% <0%> (ø) ⬆️
lib/ui/src/modules/ui/configs/handle_keyevents.js 33.33% <0%> (ø) ⬆️
app/react/src/client/preview/client_api.js 39.28% <0%> (ø) ⬆️
addons/info/src/components/markdown/htags.js 0% <0%> (ø) ⬆️
... and 12 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update acfddc0...b3d79a5. Read the comment docs.

@ndelangen
Copy link
Member

Thank you for this PR @thomasthesecond !

Will run it locally and merge today!

@ndelangen ndelangen merged commit d054507 into storybookjs:master Jul 7, 2017
@thomasthesecond thomasthesecond deleted the fix-addon-actions branch July 7, 2017 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants