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

CLI add command #4236

Merged
merged 11 commits into from
Jan 19, 2019
Merged

CLI add command #4236

merged 11 commits into from
Jan 19, 2019

Conversation

Keraito
Copy link
Contributor

@Keraito Keraito commented Sep 27, 2018

Issue: More CLI enhancements.

What I did

Added an add command to the storybook CLI to add addons to the Storybook project.

How does it work

  1. Syntax: storybook add <addon>. This can be either an official storybook addon like knobs or a random npm package. It can also include a tag like knobs@alpha, as long as it works with yarn or npm.
  2. Check if it's an official Storybook addon by running yarn info @storybook/addons-<addon> version (or npm equivalent).
    • If not: check if it's an npm package by executing yarn info <addon> version.
      • If not: display error that it's an invalid addon/npm package.
      • If so: proceed to step 3 with isOfficialAddon = false.
    • If so: proceed to step 3 isOfficialAddon = true.
  3. Check isOfficialAddon:
    • If it's false: install it as an devDependency by running yarn add -D <addon>*.
    • If it's true: Check the package.json for installed Storybook packages.
      • If any found: take the version of the first** package found and run yarn add -D @storybook/addons-<addon>@<version>.
      • If none found: run yarn add -D @storybook/addons-<addon>.
  4. Register the installed addon by read the .storybook/addons.js and put a register statement after the last register occurrence.

*Maybe adjusting the package.json and then running yarn/npm install is a better long term solution?
**Works under the assumption that the user has the same versions for every installed official Storybook package. My plan is to add some checks on this and warn the user about it in the future.

Please tell me what you guys think, both feature and implementation wise!

@Keraito Keraito added this to the next milestone Sep 27, 2018
@Keraito Keraito self-assigned this Sep 27, 2018
@Keraito Keraito requested a review from ndelangen September 27, 2018 04:32
@Keraito
Copy link
Contributor Author

Keraito commented Sep 27, 2018

image

I wouldn't really consider this duplicate code of ~18 lines, CodeFactor 😨

@ndelangen
Copy link
Member

It's fine

@Keraito Keraito changed the title Cli add CLI add command Sep 27, 2018
@codecov
Copy link

codecov bot commented Sep 30, 2018

Codecov Report

Merging #4236 into next will increase coverage by 0.04%.
The diff coverage is 38.09%.

Impacted file tree graph

@@            Coverage Diff             @@
##             next    #4236      +/-   ##
==========================================
+ Coverage   35.05%   35.09%   +0.04%     
==========================================
  Files         593      594       +1     
  Lines        7348     7408      +60     
  Branches     1000     1016      +16     
==========================================
+ Hits         2576     2600      +24     
- Misses       4259     4290      +31     
- Partials      513      518       +5
Impacted Files Coverage Δ
lib/cli/bin/generate.js 0% <0%> (ø) ⬆️
lib/cli/lib/add.js 40% <40%> (ø)
lib/cli/lib/has_yarn.js 0% <0%> (ø) ⬆️

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 b359bb2...bd421eb. Read the comment docs.

@stale
Copy link

stale bot commented Nov 3, 2018

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 Nov 3, 2018
@Hypnosphi Hypnosphi changed the base branch from master to next November 5, 2018 17:04
@stale stale bot removed the inactive label Nov 5, 2018
@stale
Copy link

stale bot commented Nov 26, 2018

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 Nov 26, 2018
@Keraito Keraito removed the inactive label Dec 22, 2018
@Keraito
Copy link
Contributor Author

Keraito commented Dec 22, 2018

Trying to pick things up again from where I left off 😄 @shilman @ndelangen could I get your thoughts/reviews on this PR? Maybe we can also talk on Discord about what we wanted to work on regarding the CLI.

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.

Looks great!

@shilman
Copy link
Member

shilman commented Dec 23, 2018

@Keraito Can you resolve the conflicts?

@Keraito
Copy link
Contributor Author

Keraito commented Dec 23, 2018

will do @shilman !

@Keraito
Copy link
Contributor Author

Keraito commented Dec 26, 2018

Should be good now @shilman

Copy link
Member

@wuweiweiwu wuweiweiwu left a comment

Choose a reason for hiding this comment

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

NICE!

@stale
Copy link

stale bot commented Jan 19, 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 Jan 19, 2019
@Hypnosphi Hypnosphi merged commit 9811dc7 into storybookjs:next Jan 19, 2019
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.

6 participants