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

Replaced deprecated markdown-to-react-components with marksy #1188

Merged
merged 3 commits into from
Jun 5, 2017

Conversation

ranneyd
Copy link
Contributor

@ranneyd ranneyd commented Jun 5, 2017

Issue: #1114

What I did

Looking at issue 1114 (#1114)...
The error came from a package called markdown-to-react-components. Looking at this package's github (https://github.com/christianalfoni/markdown-to-react-components) it's clear that the package is deprecated. They recommend using marksy (https://github.com/cerebral/marksy).

Marksy appears to be a fork of markdown-to-react-components because the APIs are almost identical (namely, the configuration is exactly the same). This made swapping out the old version for the new one very easy, which is what I did. I also updated the name of the config (which can be passed as an argument to this addon) to reflect the new underlying mechanism.

How to test

  1. Create a simple Storybook project with info (use the readme of @storybook/addon-info for helP)
  2. Make a story for a component WITH PROPTYPES DEFINED.
  3. Confirm that when you run your storybook, clicking on "Show Info" at the top shows the correct PropType info.

@shilman
Copy link
Member

shilman commented Jun 5, 2017

Thanks @ranneyd for diagnosing the problem and then fixing it. You rock! 👊

Edited the PR description slightly. Code looks great. I'll test it out on my machine and merge if it works.

Many thanks!

@shilman shilman changed the title Issue 1114: Replacing use of deprecated markdown-to-react-components … Replaced deprecated markdown-to-react-components with marksy Jun 5, 2017
@codecov
Copy link

codecov bot commented Jun 5, 2017

Codecov Report

Merging #1188 into master will not change coverage.
The diff coverage is 0%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1188   +/-   ##
=======================================
  Coverage   13.31%   13.31%           
=======================================
  Files         199      199           
  Lines        4588     4588           
  Branches      531      540    +9     
=======================================
  Hits          611      611           
+ Misses       3501     3492    -9     
- Partials      476      485    +9
Impacted Files Coverage Δ
addons/info/src/index.js 0% <ø> (ø) ⬆️
addons/info/src/components/Story.js 0% <0%> (ø) ⬆️
app/react-native/src/bin/storybook-start.js 0% <0%> (ø) ⬆️
addons/knobs/src/components/types/Object.js 4.81% <0%> (ø) ⬆️
addons/storyshots/src/storybook-channel-mock.js 0% <0%> (ø) ⬆️
app/react/src/client/preview/error_display.js 0% <0%> (ø) ⬆️
...rc/modules/ui/components/left_panel/text_filter.js 33.33% <0%> (ø) ⬆️
app/react/src/client/preview/reducer.js 0% <0%> (ø) ⬆️
addons/knobs/src/KnobManager.js 39.13% <0%> (ø) ⬆️
lib/ui/src/modules/ui/configs/init_panels.js 25% <0%> (ø) ⬆️
... and 16 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 d59feed...48b2c40. Read the comment docs.

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ranneyd Looks like you need to update package.json to add marksy and remove markdown-to-react-components?

@ranneyd
Copy link
Contributor Author

ranneyd commented Jun 5, 2017

facepalm

I actually had but I accidentally undid that change. Fixing now.

@ranneyd
Copy link
Contributor Author

ranneyd commented Jun 5, 2017

@shilman try it now. Let me know how it goes!

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ranneyd Figured you must have made the changes at some point to get this working. Looks great and thanks so much for contributing! 🏅 Merging.

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.

3 participants