Better handling of undetected project types #1647

Hypnosphi · 2017-08-12T18:08:09Z

Fixes #475

Currently, we are not really helpful when we can't detect project type. This PR adds a note about slow start guildes and offers options for bootstrapping a project

Hypnosphi · 2017-08-12T18:08:38Z

Any help with wording is welcome =)

usulpro · 2017-08-12T19:58:32Z

Do we need to check that project folder is empty and provide this options only in that case?

Hypnosphi · 2017-08-12T20:53:34Z

Not really, as all the boilerplates create a new folder

codecov · 2017-08-18T01:26:59Z

Codecov Report

Merging #1647 into release/3.3 will decrease coverage by 1.39%.
The diff coverage is 0%.

@@              Coverage Diff               @@
##           release/3.3    #1647     +/-   ##
==============================================
- Coverage        22.46%   21.06%   -1.4%     
==============================================
  Files              321      252     -69     
  Lines             6276     5748    -528     
  Branches           787      685    -102     
==============================================
- Hits              1410     1211    -199     
+ Misses            4275     4015    -260     
+ Partials           591      522     -69

Impacted Files	Coverage Δ
lib/cli/lib/helpers.js	`0% <0%> (ø)`	⬆️
lib/cli/bin/generate.js	`0% <0%> (ø)`	⬆️
...ts/stories/required_with_context/Button.stories.js	`0% <0%> (-100%)`	⬇️
...dons/storyshots/stories/directly_required/index.js	`0% <0%> (-100%)`	⬇️
...s/stories/required_with_context/Welcome.stories.js	`0% <0%> (-100%)`	⬇️
addons/storyshots/src/index.js	`0% <0%> (-80.86%)`	⬇️
app/vue/src/server/babel_config.js	`0% <0%> (-77.42%)`	⬇️
addons/storyshots/src/require_context.js	`0% <0%> (-45.59%)`	⬇️
addons/storyshots/src/storybook-channel-mock.js	`0% <0%> (-22.23%)`	⬇️
addons/storyshots/src/test-bodies.js	`0% <0%> (-22.23%)`	⬇️
... and 191 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 5e78851...b293f63. Read the comment docs.

danielduan

We should really tell the users if we're going to install something. I think it might be helpful to go one more step and ask a [y/N] to see if the user wants to install the package if it's not currently installed too.

danielduan · 2017-08-20T22:38:12Z

lib/cli/bin/generate.js

+
+    default:
+      paddedLog(`We couldn't detect your project type. (code: ${projectType})`);
+      paddedLog('Check that you are at your project directory.');


Please make sure you are running the 'getstorybook' command in your project's root directory. maybe?

danielduan · 2017-08-20T22:38:53Z

lib/cli/bin/generate.js

+      paddedLog(`We couldn't detect your project type. (code: ${projectType})`);
+      paddedLog('Check that you are at your project directory.');
+      paddedLog(
+        'You can bootstrap a project using some of the options below, or follow one of the slow start guides: https://storybook.js.org/basics/slow-start-guide/'


You can also start a project using ...

create a project maybe?

danielduan · 2017-08-20T22:41:54Z

lib/cli/bin/generate.js

+          {
+            type: 'list',
+            name: 'type',
+            message: 'Which kind of project would you like to bootstrap?',


Which kind of project boilerplate would you like to install and start?

I think install is pretty key here since we are going installing something into the user's system without their acknowledgement here.

Unless it's yarn create, in which case the installation will be temporary

shilman

Why are we globally installing anything on the user's behalf at all? AFAIK we shouldn't be.

Hypnosphi · 2017-08-20T23:46:03Z

Why are we globally installing anything on the user's behalf at all? AFAIK we shouldn't be.

So what would you offer instead? Just print command-line instructions for those boilerplates?

shilman · 2017-08-21T00:01:24Z

@Hypnosphi we definitely shouldn't be installing things globally on the user's behalf. I think we can either print out instructions or better yet direct them to a docs page where we offer some instructions.

danielduan · 2017-08-21T00:10:04Z

I can see how running create-react-app for the user if they have it installed is a potentially valid use case, but ideally I don't think we should be installing anything for the user either. Either they have it installed and we run it for them, or we give them instructions to install and have them rerun getstorybook.

Hypnosphi · 2017-08-21T00:12:29Z

If the tool offers me an option to run some routine steps for me, and is explicit when it comes to global stuff (see latest commit), why is this something wrong?

In my view it's far better UX when comparing to "copy and paste URL from terminal to browser, and then a bunch of commands back from browser to terminal"

In fact we can even offer to uninstall those as soon as we're done, similar to what yarn create and npx do

usulpro · 2017-08-21T00:23:02Z

what do you think about npx --no-install as a workaround?

Hypnosphi · 2017-08-21T00:27:20Z

npx --no-install as a workaround

I like the idea. If either npx or the package isn't available, we can fall back to plain command line instructions

usulpro · 2017-08-21T00:36:05Z

we can have npx as a dependency of getstorybook right? so user don't need to have it

ps sorry, don't need --no-install flag in our case
https://github.com/zkat/npx

shilman · 2017-08-21T01:45:18Z

Why do we want to get into the business of creating projects for users? It's perfectly valid for getstorybook to have a set of projects that it's compatible with, and to redirect people to instructions when their project isn't compatible.

Hypnosphi · 2017-08-21T11:05:17Z

@shilman because of this

ndelangen · 2017-08-22T20:59:07Z

We should not generate project scaffolds ourselves only append what is storybook related.
If we do this, we should use a generator without installing it permanently or globally.

But should we do this?
I have to say I am conflicted on this. From a user-perspective this could be very nice.
For a maintainer's perspective this could be a nightmare: keeping in sync with a lot of generators could be a world of pain.

On the other hand, we claim to be compatible with these generators (we currently are). So maybe we SHOULD do something like this to keep us alert and compatible.

Hypnosphi · 2017-09-09T01:35:44Z

Replaced with #1825

@ndelangen's concern about compatibility with latest generators versions seem to be solved with #1767

Hypnosphi and others added 2 commits August 12, 2017 21:04

Better handling of undetected project types

48b225e

Merge branch 'master' into undetected-options

b3f2587

Hypnosphi added the cli label Aug 12, 2017

Hypnosphi added the feature request label Aug 13, 2017

Merge branch 'master' into undetected-options

76e7227

Hypnosphi and others added 4 commits August 18, 2017 12:47

Merge branch 'master' into undetected-options

4cd72df

Merge branch 'master' into undetected-options

ca30281

Merge branch 'master' into undetected-options

a78fdf7

Merge branch 'master' into undetected-options

da23ec2

Hypnosphi requested a review from a team August 20, 2017 10:53

Hypnosphi added 2 commits August 20, 2017 14:22

Merge branch 'master' into undetected-options

5f7a835

Merge branch 'master' into undetected-options

0374271

danielduan requested changes Aug 20, 2017

View reviewed changes

shilman requested changes Aug 20, 2017

View reviewed changes

Adjust wording. Add a warning about installing global packages

b293f63

ndelangen added the discussion label Aug 22, 2017

ndelangen assigned Hypnosphi Aug 22, 2017

Hypnosphi mentioned this pull request Aug 27, 2017

Do a proper full test of CLI in CI #1746

Closed

usulpro mentioned this pull request Aug 28, 2017

Storybook v3 storybook-eol/react-cdk#31

Open

Hypnosphi changed the base branch from master to release/3.3 September 9, 2017 01:12

Hypnosphi mentioned this pull request Sep 9, 2017

CLI: A more human-friendly message for undetected project types #1825

Merged

Hypnosphi closed this Sep 9, 2017

ndelangen deleted the undetected-options branch September 9, 2017 07:28

This was referenced Jan 12, 2018

puppeteer versions available: 1.0.0 #2731

Closed

puppeteer versions available: 1.0.0 #2732

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Better handling of undetected project types #1647

Better handling of undetected project types #1647

Hypnosphi commented Aug 12, 2017

Hypnosphi commented Aug 12, 2017

usulpro commented Aug 12, 2017

Hypnosphi commented Aug 12, 2017

codecov bot commented Aug 18, 2017 •

edited

Loading

danielduan left a comment

danielduan Aug 20, 2017 •

edited

Loading

danielduan Aug 20, 2017

Hypnosphi Aug 20, 2017

danielduan Aug 20, 2017

Hypnosphi Aug 20, 2017

shilman left a comment

Hypnosphi commented Aug 20, 2017

shilman commented Aug 21, 2017

danielduan commented Aug 21, 2017 •

edited

Loading

Hypnosphi commented Aug 21, 2017 •

edited

Loading

usulpro commented Aug 21, 2017

Hypnosphi commented Aug 21, 2017 •

edited

Loading

usulpro commented Aug 21, 2017

shilman commented Aug 21, 2017

Hypnosphi commented Aug 21, 2017

ndelangen commented Aug 22, 2017

Hypnosphi commented Sep 9, 2017

Better handling of undetected project types #1647

Better handling of undetected project types #1647

Conversation

Hypnosphi commented Aug 12, 2017

Hypnosphi commented Aug 12, 2017

usulpro commented Aug 12, 2017

Hypnosphi commented Aug 12, 2017

codecov bot commented Aug 18, 2017 • edited Loading

Codecov Report

danielduan left a comment

Choose a reason for hiding this comment

danielduan Aug 20, 2017 • edited Loading

Choose a reason for hiding this comment

danielduan Aug 20, 2017

Choose a reason for hiding this comment

Hypnosphi Aug 20, 2017

Choose a reason for hiding this comment

danielduan Aug 20, 2017

Choose a reason for hiding this comment

Hypnosphi Aug 20, 2017

Choose a reason for hiding this comment

shilman left a comment

Choose a reason for hiding this comment

Hypnosphi commented Aug 20, 2017

shilman commented Aug 21, 2017

danielduan commented Aug 21, 2017 • edited Loading

Hypnosphi commented Aug 21, 2017 • edited Loading

usulpro commented Aug 21, 2017

Hypnosphi commented Aug 21, 2017 • edited Loading

usulpro commented Aug 21, 2017

shilman commented Aug 21, 2017

Hypnosphi commented Aug 21, 2017

ndelangen commented Aug 22, 2017

Hypnosphi commented Sep 9, 2017

codecov bot commented Aug 18, 2017 •

edited

Loading

danielduan Aug 20, 2017 •

edited

Loading

danielduan commented Aug 21, 2017 •

edited

Loading

Hypnosphi commented Aug 21, 2017 •

edited

Loading

Hypnosphi commented Aug 21, 2017 •

edited

Loading