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

Better "module not found" message #401

Closed
gaearon opened this issue Aug 8, 2016 · 14 comments
Closed

Better "module not found" message #401

gaearon opened this issue Aug 8, 2016 · 14 comments

Comments

@gaearon
Copy link
Contributor

gaearon commented Aug 8, 2016

This is what I see when I forget to install some npm dependencies:

screen shot 2016-08-08 at 16 34 37

wat

This is overwhelming and unnecessary.
We should have simple messages, like:

These dependencies were not found in node_modules:

  * redux
  * react-redux

Did you forget to run npm install --save for them?

I’m not sure what to do with local file missing errors (i.e. when you import a non-existing file in src). We should probably group them separately but also in a less intimidating way.

I don’t personally plan to work on this so any help is welcome!

@gaearon
Copy link
Contributor Author

gaearon commented Aug 8, 2016

If you plan to work on this please leave a comment. We don’t want many people to work on the same thing at the same time.

A good place to start looking at would be scripts/start.js. We already handle Webpack errors in an unorthodox way so we may want to keep refining that.

cc @geowarin who also was looking into improvements to webpack output in geowarin/tarec#16.

@rogeliog
Copy link
Contributor

rogeliog commented Aug 8, 2016

@gaearon I would love to work on this one 😄

@geowarin
Copy link
Contributor

geowarin commented Aug 8, 2016

Hi @gaearon.

If you change this line to var json = stats.toJson({}, true);, you'll get a much shorter error output.
It is what webpack normally displays on errors (ie, when you don't parse the errors yourself) and it doesn't include the details field.
See here.

This is a short term measure before somebody actually dives into webpack internals to do exactly what you suggested.
(I'm working on geowarin/tarec#16 and as promised I will contribute back when it's ready but if somebody wants to tackle it in the meantine, it would be interesting)

gaearon added a commit that referenced this issue Aug 8, 2016
@jnsdls
Copy link

jnsdls commented Aug 8, 2016

this would be a great change not just for this specific project but in general (at least as an option)

geowarin added a commit to geowarin/tarec that referenced this issue Aug 9, 2016
@rogeliog
Copy link
Contributor

rogeliog commented Aug 9, 2016

Seems that @geowarin already has a PR 👍, he also has more domain knowledge than me!

@geowarin
Copy link
Contributor

I created the friendly-errors-webpack-plugin.

It still needs more work (see the TODO in the README) but I'd love to see your feedback/ PR. 😄
ETA: Friday night

@rogeliog
Copy link
Contributor

@geowarin Awesome, given that you created friendly-errors-webpack-plugin I thought that it would make more sense me to PR that repo instead of this one. geowarin/friendly-errors-webpack-plugin#1

@rogeliog
Copy link
Contributor

@geowarin is there an update on this? I'm more than happy to pick it up or help out with anything :)

gaearon added a commit that referenced this issue Aug 22, 2016
@fson
Copy link
Contributor

fson commented Aug 30, 2016

What if instead of

Did you forget to run npm install --save for them?

missing packages would bring out a prompt to install them for you?

Would you like to install the missing packages? [Y/n]

@geowarin
Copy link
Contributor

Update: there is a PR implementing this.

@fson It would be great to provide autofixes! The only part I'm not sure about is how to allow simple interactions with the console since it is cleared every now and then.

@gaearon
Copy link
Contributor Author

gaearon commented Aug 31, 2016

I’m a bit worried about autofixes because it’s easier to miss typos and install something you didn’t want. I’d like installation act to be explicit.

stayradiated pushed a commit to stayradiated/create-react-app that referenced this issue Sep 7, 2016
@mareksuscak
Copy link
Contributor

@gaearon As per Heroku's Node.js best practices we could mitigate the issue by providing .npmrc in the project root folder for newly created projects with essential defaults. We've been using that technique for quite a while now and it's certainly lowered the number of those "forgotten to --save the dependency" issues. If interested I could work on that part.

This is what we use:

save=true
save-exact=true

@fson
Copy link
Contributor

fson commented Sep 11, 2016

@mareksuscak Thanks for the tip, didn't know about this setting before. However, I think it could increase the confusion in some cases, because we would basically be creating a hidden file that makes npm behave differently from the default. People might not notice it or understand what it does, only npm would seem to behave differently from time to time. That's why I think a simple message showing how to use npm install --save, as suggested originally, would be a better option.

This project also aims to avoid adding configuration and flags as much as possible, so generating a .npmrc file would not fit well in this philosophy.

feiqitian pushed a commit to feiqitian/create-react-app that referenced this issue Oct 25, 2016
@gaearon
Copy link
Contributor Author

gaearon commented Mar 7, 2017

I think this was fixed.

@gaearon gaearon closed this as completed Mar 7, 2017
cszatmary pushed a commit to cszatmary/create-react-app that referenced this issue Dec 6, 2018
increased support for common image formats
@lock lock bot locked and limited conversation to collaborators Jan 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants