-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
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
Print user friendly error if no entrypoint can be found #1848
Conversation
src/Bundler.js
Outdated
process.exitCode = 1; | ||
if (process.env.NODE_ENV === 'production' || !initialised) { | ||
await this.stop(); | ||
process.exit(1); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Apparently process.exitCode = 1;
didn't exit the process anymore for some reason, so changed this to be a lil more aggresive and added await this.stop();
to make sure servers and workers shutdown properly
src/Bundler.js
Outdated
this.buildQueue.add(asset); | ||
this.entryAssets.add(asset); | ||
} catch (err) { | ||
throw new Error(`Failed to load entrypoint: ${entry}!`); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think of a more precise error message, something like: Failed to resolve entrypoint "${entry}" in "${rootDir}"
Given that we infer rootDir
I think it'd be better to give users more info so they can debug why we can't find their entrypoint.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure thing, I'll update it
src/Bundler.js
Outdated
throwErrors: | ||
typeof options.throwErrors === 'boolean' | ||
? options.throwErrors | ||
: process.env.NODE_ENV === 'test' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should default this to false
in CLI and true
when using the Bundler
API.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably, I'll update it
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be considered a breaking change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just something I thought about, parcel already throws errors in the api using the event handler.
So this might cause issues as the promise of bundle only returns on initial build. What do you think?
src/Bundler.js
Outdated
this.entryAssets.add(asset); | ||
} catch (err) { | ||
throw new Error( | ||
`Failed to load entrypoint: "${entry}" in "${ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would say "Cannot resolve entry ${entry}". As of the error we haven't actually tried to load anything yet, just resolved.
Print a user friendly error message if no entrypoint can be found.
Also prints a user friendly error if a user defined entrypoint has a resolve error
Closes #1797 Closes #1829