Show warning message when entry file not provided (#1797)

[rajikaimal]

Fixes #1797

[DeMoorJasper]

This doesn't check for main in pkg or any other resolving that might happen?

I originally thought you were gonna catch the error and just print a user-friendly one instead (if not entry if found)?

[DeMoorJasper]

This doesn't check for main in pkg or any other resolving that might happen?

I originally thought you were gonna catch the error and just print a user-friendly one instead (if not entry if found)?

[rajikaimal]

@DeMoorJasper According to my understanding none of the entry points you mentioned are not checking for main in pkg.json, can you point me to the place where it checks if such a condition exists? Sorry for the hassle caused 😅

[DeMoorJasper]

It's still checking it, but it shouldn't throw a warning. Parcel can be used without entrypoint it's optional, it should just throw a user friendly error if it can't find one.

[mohsen1]

@rajikaimal I think @DeMoorJasper means package.json main field, not a file named pkg.json :)

[rajikaimal]

@mohsen1 Yeah that's what I meant by pkg.json.

@DeMoorJasper Isn't a warning log the best for this scenario, rather than an error log?

[DeMoorJasper]

Users of parcel are allowed to use non explicit entrypoints at their own risk, we can log the entrypoints if that's really necessary.

But the most important thing is if parcel can't resolve an entrypoint it should throw a nice error, otherwise users think it's a parcel bug or something. Because users expect too much magic.

I'm basically talking about wrapping this
(https://github.com/parcel-bundler/parcel/blob/master/src/Bundler.js#L245) in a try {} catch {} and throwing a user friendly error telling the entrypoint could not be resolved.

The error should probably also exit the parcel process (as parcel can't do anything without), not sure how that would be achieved

[jchiatt]

@DeMoorJasper Is this what you're looking for?

for (let entry of this.entryFiles) {
  try {
    let asset = await this.resolveAsset(entry);
    this.buildQueue.add(asset);
    this.entryAssets.add(asset);
  } catch (err) {
    const errMsg = `Failed to load entrypoint. ${err}`
    logger.error(errMsg);
    process.exit(1);
  }
}

[DeMoorJasper]

Sort of the process.exit seems a bit hacky to me though as in the catch of build we already have something similar

_{Sent with GitHawk}

[jchiatt]

@DeMoorJasper Copy that. I don't have a lot of familiarity with exiting processes - maybe someone else can chime in

[rajikaimal]

@jchiatt Hey, did you test the code with try-catch? This didn't resolve the bug for me.

[DeMoorJasper]

@rajikaimal what bug? User friendly errors are a feature. In theory the try {} catch {} should work

Personally I would probably create something like this:

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

	// Scary error throwing
	throw err;	

	// Or use the logger
	logger.error(err);
	process.exit(1);
  }
}

Throwing an error will output a fairly big error message, this might be too scary? You can try it out with the logger it should work in theory, as it's a singleton. It should print errors by default.

But definitely change the error to show Failed to load entrypoint: ${entry}, so that the user knows which entry failed in case it's an explicit defined entry

[rajikaimal]

"what bug?" Misinterpretation should have mentioned as "the issue".

Anyways execution doesn't go to catch block at all.

[DeMoorJasper]

That’s strange than it can resolve it right?
If it did not throw an error that would make sense but than it actually worked

Might be that the resolver just doesn’t check a file exists

[DeMoorJasper]

I've opened a PR with what I had in mind, see #1848