Ensure CLI warnings still show on errors, add an error for externalId…

… collissions (#2334)

* ensure CLI warnings display on error, ensure external doesnt collide with existing

* throw when an external collides with a non-external

* include test for external colissions

bin/src/run/build.ts

@@ -82,5 +82,8 @@ export default function build(
 				printTimings(bundle.getTimings());
 			}
 		})
-		.catch(handleError);
+		.catch((err: any) => {
+			if (warnings.count > 0) warnings.flush();
+			handleError(err);
+		});
 }

src/Graph.ts

@@ -837,6 +837,15 @@ Try defining "${chunkName}" first in the manualChunks definitions of the Rollup
 
 							const externalModule = this.moduleById.get(externalId);
 
+							if (externalModule instanceof ExternalModule === false) {
+								error({
+									code: 'INVALID_EXTERNAL_ID',
+									message: `'${source}' is imported as an external by ${relativeId(
+										module.id
+									)}, but is already an existing non-external module id.`
+								});
+							}
+
 							// add external declarations so we can detect which are never used
 							for (const name in module.imports) {
 								const importDeclaration = module.imports[name];

test/function/samples/external-conflict/_config.js

@@ -0,0 +1,25 @@
+var path = require('path');
+var assert = require('assert');
+
+module.exports = {
+	description: 'external paths from custom resolver remain external (#633)',
+	options: {
+		external: (_id, parent) => parent === 'dep',
+		plugins: [
+			{
+				resolveId (id, parent) {
+					if (id === 'dep')
+						return id;
+				},
+				load (id) {
+					if (id === 'dep')
+						return `import 'dep'`;
+				}
+			}
+		]
+	},
+	error: {
+		code: "INVALID_EXTERNAL_ID",
+		message: "'dep' is imported as an external by dep, but is already an existing non-external module id."
+	}
+};

test/function/samples/external-conflict/main.js

@@ -0,0 +1 @@
+import 'dep';