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.

Sign up for GitHub

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

Jump to bottom

Print user friendly error if no entrypoint can be found #1848

Merged
merged 5 commits into from Aug 23, 2018
Merged

Print user friendly error if no entrypoint can be found #1848

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
4048a31
print user friendly error if no entrypoint can be found
DeMoorJasper Aug 3, 2018
2b191f4
remove useless isInitial check
DeMoorJasper Aug 3, 2018
ea56814
fix bundler error throwing
DeMoorJasper Aug 10, 2018
8c3ace7
improve error message and always throw in non-cli environment
DeMoorJasper Aug 16, 2018
7bb53cb
Update Bundler.js
devongovett Aug 23, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
30 changes: 23 additions & 7 deletions src/Bundler.js
View file
Open in desktop
Expand Up @@ -142,7 +142,11 @@ class Bundler extends EventEmitter {
contentHash:
typeof options.contentHash === 'boolean'
? options.contentHash
: isProduction
: isProduction,
throwErrors:
typeof options.throwErrors === 'boolean'
? options.throwErrors
: process.env.NODE_ENV === 'test'
Copy link
Contributor

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.

Copy link
Member Author

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

Copy link
Member Author

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?

Copy link
Member Author

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?

};
}

Expand Down Expand Up @@ -223,6 +227,7 @@ class Bundler extends EventEmitter {

let isInitialBundle = !this.entryAssets;
let startTime = Date.now();
let initialised = !isInitialBundle;
this.pending = true;
this.error = null;

Expand All @@ -242,10 +247,20 @@ class Bundler extends EventEmitter {

this.entryAssets = new Set();
for (let entry of this.entryFiles) {
let asset = await this.resolveAsset(entry);
this.buildQueue.add(asset);
this.entryAssets.add(asset);
try {
let asset = await this.resolveAsset(entry);
this.buildQueue.add(asset);
this.entryAssets.add(asset);
} catch (err) {
throw new Error(`Failed to load entrypoint: ${entry}!`);
Copy link
Contributor

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.

Copy link
Member Author

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

}
}

if (this.entryAssets.size === 0) {
throw new Error('No entrypoint(s) found.');
}

initialised = true;
}

// Build the queued assets.
Expand Down Expand Up @@ -318,10 +333,11 @@ class Bundler extends EventEmitter {
this.hmr.emitError(err);
}

if (process.env.NODE_ENV === 'production') {
process.exitCode = 1;
} else if (process.env.NODE_ENV === 'test' && !this.hmr) {
if (this.options.throwErrors && !this.hmr) {
throw err;
} else if (process.env.NODE_ENV === 'production' || !initialised) {
await this.stop();
process.exit(1);
Copy link
Member Author

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

}
} finally {
this.pending = false;
Expand Down
3 changes: 2 additions & 1 deletion test/utils.js
View file
Open in desktop
Expand Up @@ -37,7 +37,8 @@ function bundler(file, opts) {
cache: false,
killWorkers: false,
hmr: false,
logLevel: 0
logLevel: 0,
throwErrors: true
},
opts
)
Expand Down