joshwcomeau / guppy

🐠A friendly application manager and task runner for React.js
ISC License
3.27k stars 154 forks source link

react-scripts not found #330

Closed AWolf81 closed 5 years ago

AWolf81 commented 5 years ago

During my work on #260 I've noticed an issue that can't be fixed in Guppy right now & Guppy doesn't display anything what's wrong.

Describe the bug If node_modules folder is missing there will be an error message that react-scripts can't be found. This will happen on projects cloned from Github or if you restore a previously deleted project from recycle bin and re-import it to Guppy. To reinstall node_modules we're adding a feature see PR #318.

To Reproduce

  1. Create a CRA project
  2. Open the project folder in Finder/Explorer & delete node_modules folder
  3. Reload Guppy (just to see that there are no dependency but this step is optional)
  4. Any started task will fail with the erro message from the screenshot below

Expected behavior On any task run with-out node_modules folder, display an error dialog that's giving details to the issue & bail early. The info will give details how to fix the problem. We could also display a button in the dialog so node_modules can be reinstalled from that notification. But I think that's optional as there will be a button in the UI.

Screenshots error message

Environment:

Todos

menor commented 5 years ago

I tried to reproduce but got a different error, probably because I have a global version of react-scripts installed. I guess this does not affect this issue since we are checking if node_module folder exists, but posting it just in case.

image
melanieseltzer commented 5 years ago

@menor Yeah I think because you have react-scripts installed globally, it probably moves on to the next module that it finds missing (I'm speculating but it would make sense).

For the original suggestions in the issue...

On any task run with-out node_modules folder, display an error dialog that's giving details to the issue & bail early.

We could also display a button in the dialog so node_modules can be reinstalled from that notification. But I think that's optional as there will be a button in the UI.

@AWolf81 I don't think these will be necessary anymore right? Because all Task panes will now be hidden completely when #318 lands. Or is there another case that would require this?

AWolf81 commented 5 years ago

@melanieseltzer Yes, you're right. There is no other case. Once reinstall lands this is not needed as any button will be hidden and we're not running into this issue.

I think we can close this once reinstall is merged.

AWolf81 commented 5 years ago

Closing as we're checking that all dependencies are installed. Reinstall feature merged.