fix!: Return a non-zero error code on errors in build, lint (#982)

* Return a non-zero error codes on errors in build, lint

* Fix lint error in nominal-connection-checker

* Fix build issue in block-extension-tooltip

* Update Blockly version in block-extension-tooltip

* fix: don't update version of blockly in block-extension-tooltip

* chore: fix lint error, we allow long import lines now

* fix: ignore warnings from source-map loader

* fix: ignore warnings from source-map loader

* chore: remove outdated example directory of workspace-search

* feat: show all lint messages from both src and test of a plugin before exiting if there's an error in either

* feat: have lerna not exit early if it encounters a lint error in a package

* fix: return type in lint script, and added some comments

Co-authored-by: Maribeth Bottorff <maribethb@google.com>

package.json

@@ -16,7 +16,7 @@
         "deploy": "npm run deploy:prepare && gulp deploy",
         "deploy:upstream": "npm run deploy:prepare && gulp deployUpstream",
         "license": "gulp checkLicenses",
-        "lint": "lerna run lint",
+        "lint": "lerna run lint --no-bail",
         "lint:fix": "lerna run lint -- -- --fix",
         "postinstall": "npm run boot",
         "test": "lerna run test",

plugins/block-extension-tooltip/src/index.ts

@@ -15,7 +15,7 @@
 import * as Blockly from 'blockly';
 import './tooltip_monkey_patch';
 
-type TooltipRender = (block: Blockly.BlockSvg) => HTMLElement;
+type TooltipRender = (block: Blockly.BlockSvg) => HTMLDivElement;
 
 interface TooltipBlock extends Blockly.BlockSvg {
   customTooltip: () => HTMLElement;

plugins/block-test/src/index.js

@@ -14,15 +14,12 @@ import * as Blockly from 'blockly/core';
 
 import {category as alignCategory, onInit as initAlign} from './align';
 import {category as basicCategory, onInit as initBasic} from './basic';
-import {
-  category as connectionsCategory, onInit as initConnections,
-} from './connections';
+import {category as connectionsCategory, onInit as initConnections} from './connections';
 import {category as dragCategory, onInit as initDrag} from './drag';
 import {category as fieldsCategory, onInit as initFields} from './fields';
 import {category as mutatorsCategory, onInit as initMutators} from './mutators';
 import {category as styleCategory, onInit as initStyle} from './style';
-import {category as serializationCategory, onInit as initSerialization} from
-  './serialization/category';
+import {category as serializationCategory, onInit as initSerialization} from './serialization/category';
 
 
 /**

plugins/dev-scripts/config/webpack.config.js

@@ -136,6 +136,9 @@ module.exports = (env) => {
         },
       ].filter(Boolean),
     },
+    // Ignore spurious warnings from source-map-loader
+    // It can't find source maps for some Closure modules and that is expected
+    ignoreWarnings: [/Failed to parse source map/],
     plugins: [
       // Add package name.
       new webpack.DefinePlugin({

plugins/dev-scripts/scripts/build.js

@@ -45,15 +45,33 @@ Make sure a ${chalk.red('src/index.(js|ts)')} file is included in your package.
 
 // Create and run the webpack compiler.
 webpack(config, (err, stats) => {
-  if (err) {
-    console.error(err.stack || err);
-    if (err.details) {
-      console.error(err.details);
-    }
-    return;
+  const statsData = stats.toJson({
+    all: false,
+    warnings: true,
+    errors: true,
+  });
+
+  const formatWebpackMessage = (obj) => {
+    return obj.message.trim();
+  };
+
+  const messages = {
+    errors: statsData.errors
+        .map(formatWebpackMessage),
+    warnings: statsData.warnings
+        .map(formatWebpackMessage),
+  };
+
+  if (!messages.errors.length && !messages.warnings.length) {
+    console.log(chalk.green('Compiled successfully!'));
+  }
+  if (messages.errors.length) {
+    console.log(chalk.red('Failed to compile.\n'));
+    console.log(messages.errors.join('\n\n'));
+    process.exit(1);
+  }
+  if (messages.warnings.length) {
+    console.log(chalk.yellow('Compiled with warnings.\n'));
+    console.log(messages.warnings.join('\n\n'));
   }
-  console.log(stats.toString({
-    chunks: false, // Makes the build much quieter
-    colors: true, // Shows colors in the console
-  }));
 });

plugins/dev-scripts/scripts/lint.js

@@ -8,6 +8,8 @@
  * @fileoverview A 'lint' script for Blockly extension packages.
  * This script:
  *   Runs eslint on the src and test directories.
+ *   If run with --fix, fixes problems that can be resolved automatically.
+ *   Returns with an error if there are any lint errors in either src or test.
  * @author samelh@google.com (Sam El-Husseini)
  */
 
@@ -42,8 +44,8 @@ const linter = new ESLint({
  * Lint this directory.
  * @param {string} dir The directory to lint.
  * @param {ESLint} linter The linter.
- * @return {Promise<Array<LintResult>|null>} The results, which may be printed
- *   with an approriate formatter.
+ * @return {Promise<Array<LintResult|Array<LintResult|null>>>} All results,
+ *   which may be printed with an approriate formatter, and error results.
  */
 async function lintDir(dir, linter) {
   const resolvePath = resolveApp(dir);
@@ -52,22 +54,34 @@ async function lintDir(dir, linter) {
     if (shouldFix) {
       await ESLint.outputFixes(results);
     }
-    return results;
+    return [results, ESLint.getErrorResults(results)];
   }
   return null;
 }
 
 linter.loadFormatter('stylish').then((formatter) => {
   // Run eslint for both the src and test directories.
-  // The eslint engine will use the .eslintrc under plugins/ for configuration.
-  lintDir('src', linter).then((result) => {
+  let exitCode = 0;
+  const src = lintDir('src', linter).then((lintResults) => {
+    const [result, errors] = lintResults;
     if (result) {
       console.log(formatter.format(result));
     }
+    if (errors.length) {
+      exitCode = 1;
+    }
   });
-  lintDir('test', linter).then((result) => {
+  const test = lintDir('test', linter).then((lintResults) => {
+    const [result, errors] = lintResults;
     if (result) {
       console.log(formatter.format(result));
     }
+    if (errors.length) {
+      exitCode = 1;
+    }
+  });
+  // Only exit on error after both directories have output their messages.
+  Promise.all([src, test]).then(() => {
+    process.exit(exitCode);
   });
 });