fix(dev-scripts)!: Fixes, refactoring and simplification of `webpack.…

…config.js` and `'blockly'` imports (#2229)

* chore(dev-scripts)!: Remove support acquiring Blockly through git

Completes revert of PR #335.

BREAKING CHANGE: This PR removes the support that was added
in PR #335 for aquiring Blockly directly from a git:// URL.

This feature was useful insofar as it enabled merging changes
into blockly-samples that depend on changes in blockly that
have not yet been published (even as a beta)—and still have
tests pass.  For this to work seamlessly, however, the code
in webpack.config.js depended on a postinstall script that
was removed in PR #1630.

When testing such PRs going forward use npm link for local
testing and wait for changes to blockly to be published
before merging the corresponding changes to samples—or wait
for blockly to become a monorepo so both changes can be made
in the same PR!

Note that this change is breaking only to the dev-scripts plugin
itself, and will not cause other plugins that use it as a dev
dependency to experience a breaking change.

* fix(dev-scripts): Restore packageJson for setting PACKAGE_NAME

The commit which removed support for git:// URLS by completing
the revert of PR #335 removed the initialisation of packageJson
(from the package.json of the plugin being built) which turns
out to still be needed by a DefinePlugin call added later.

* fix(dev-scripts): Don't use alias when resolving blockly

PR #226 addedd a resolve.alias for blockly to webpack.config.js.
It is not entirely clear what the purpose of this was at the
time, but it has the effect of treating imports of submodules
(e.g. 'blockly/core') as if they were direct imports (e.g. of
'./node_modules/blockly/core.js'), causing webpack to ignore
the blockly package's package.json file.

This causes plugins to fail to build due to the introduction
of an exports stanza in that file (and other related changes)
in google/blockly#7822.

* fix(dev-scripts): Exclude blockly from plugin bundles

Fix bloat caused by some plugins depending on all of blockly
(instead of just blockly/core), resulting in webpack including
a copy of blockly in the bundled plugin becuase only the
subpackage entrypoints were listed inthe externals stanza of
webpack.config.js.

This will also avoid certain problems that might occur due to
apps using such bundled inadvertently containing two or more
different copies of Blockly.

Also fix the one plugin which did still have an unnecessary dependency
on blockly intead of blockly/core.

* refactor(dev-scripts): Introduce exists() for readability

Currently webpack.conf.js is hard to understand.  Attempt to
improve readability by making some parts more DRY ("don't
repeat yourself") and others more DAMP ("descriptive and
 meaningful phrases").

* fix(dev-scripts): Ignore more jsdom subdependencies

Add bufferutils and utf-8-validate to IgnorePlugin config when
building tests.  These are optional dependencies of wd, which is
itself a dependency of jsdom.

Also refactor how plugins config is generated to improve readability.

* refactor(dev-scripts): Simplify resolve.extensions

There doesn't appear to be any reason not to include the '.ts'
in resolve.extensions even for pure-JS plugins, but it is
_necessary_ to include it in TS plugins.

Since the default value for resolve.extensions is
    ['.js', '.json', '.wasm']
set it to
    ['.ts', '.js', '.json', '.wasm']
which gives priority to TS, then JS, then the other default
extensions.

Also add a helpful comment explaining the purpose of
resolve.fallback.

* fix(plugins): Build tests against 'blockly', not 'blockly/node'

The latter has never been an advertised entrypoint, and will
cease to be a valid entrypoint in v11 (see google/blockly#7822).
Fortunately the 'blockly' entrypoint behaves the same as
the 'blockly/node' entrypoint does in a node.js environment.

* chore(plugins): Fix lint, formatting

* fix(dev-tools): Have runSerializationTestSuite accept Blockly

Have runSerializationTestSuite accept a second argument which is
the Blockly object to use for XML serialization/deserialization.

This works around an issue in blockly-samples that (following
the deletion of the alias for blockly in webpack.config.js)
causes tests using this function to fail due to having two copies
of jsdom loaded, where each will reject DOM objects created by
the other.  See
#2229 (comment)
for more details.

* chore(typed-variable-modal): Skip failing test

plugins/block-dynamic-connection/test/dynamic_if.mocha.js

@@ -641,5 +641,5 @@ suite('If block', function () {
       },
     },
   ];
-  testHelpers.runSerializationTestSuite(testCases);
+  testHelpers.runSerializationTestSuite(testCases, Blockly);
 });

plugins/block-dynamic-connection/test/dynamic_list_create.mocha.js

@@ -540,5 +540,5 @@ suite('List create block', function () {
       },
     },
   ];
-  testHelpers.runSerializationTestSuite(testCases);
+  testHelpers.runSerializationTestSuite(testCases, Blockly);
 });

plugins/block-dynamic-connection/test/dynamic_text_join.mocha.js

@@ -542,5 +542,5 @@ suite('Text join block', function () {
       },
     },
   ];
-  testHelpers.runSerializationTestSuite(testCases);
+  testHelpers.runSerializationTestSuite(testCases, Blockly);
 });

plugins/block-plus-minus/test/if.mocha.js

@@ -7,7 +7,7 @@
 const chai = require('chai');
 const {testHelpers} = require('@blockly/dev-tools');
 const {runPlusMinusTestSuite} = require('./test_helpers.mocha');
-const Blockly = require('blockly/node');
+const Blockly = require('blockly');
 const {dartGenerator} = require('blockly/dart');
 const {javascriptGenerator} = require('blockly/javascript');
 const {luaGenerator} = require('blockly/lua');
@@ -172,7 +172,7 @@ suite('If block', function () {
         },
       },
     ];
-    runSerializationTestSuite(testCases);
+    runSerializationTestSuite(testCases, Blockly);
   });
 
   suite('JSON', function () {
@@ -222,7 +222,7 @@ suite('If block', function () {
         },
       },
     ];
-    runSerializationTestSuite(testCases);
+    runSerializationTestSuite(testCases, Blockly);
   });
 
   runPlusMinusTestSuite('controls_if', 1, 1, 'IF', assertIfBlockStructure);

plugins/block-plus-minus/test/list_create.mocha.js

@@ -7,7 +7,7 @@
 const chai = require('chai');
 const {testHelpers} = require('@blockly/dev-tools');
 const {runPlusMinusTestSuite} = require('./test_helpers.mocha');
-const Blockly = require('blockly/node');
+const Blockly = require('blockly');
 const {dartGenerator} = require('blockly/dart');
 const {javascriptGenerator} = require('blockly/javascript');
 const {luaGenerator} = require('blockly/lua');
@@ -206,7 +206,7 @@ suite('List create block', function () {
         },
       },
     ];
-    runSerializationTestSuite(testCases);
+    runSerializationTestSuite(testCases, Blockly);
   });
 
   suite('Json', function () {
@@ -299,7 +299,7 @@ suite('List create block', function () {
         },
       },
     ];
-    runSerializationTestSuite(testCases);
+    runSerializationTestSuite(testCases, Blockly);
   });
 
   runPlusMinusTestSuite(

plugins/block-plus-minus/test/procedures.mocha.js

@@ -6,7 +6,7 @@
 
 const chai = require('chai');
 const sinon = require('sinon');
-const Blockly = require('blockly/node');
+const Blockly = require('blockly');
 const {testHelpers} = require('@blockly/dev-tools');
 const procedureTestHelpers = require('./procedures_test_helpers.mocha');
 const {dartGenerator} = require('blockly/dart');
@@ -339,7 +339,7 @@ suite('Procedure blocks', function () {
             },
           },
         ];
-        runSerializationTestSuite(testCases);
+        runSerializationTestSuite(testCases, Blockly);
       });
 
       suite('Json', function () {
@@ -514,7 +514,7 @@ suite('Procedure blocks', function () {
             },
           },
         ];
-        runSerializationTestSuite(testCases);
+        runSerializationTestSuite(testCases, Blockly);
       });
 
       suite('Adding and removing inputs', function () {

plugins/block-plus-minus/test/procedures_test_helpers.mocha.js

@@ -5,7 +5,7 @@
  */
 
 const chai = require('chai');
-const Blockly = require('blockly/node');
+const Blockly = require('blockly');
 
 require('../src/index');
 const assert = chai.assert;

plugins/block-plus-minus/test/text_join.mocha.js

@@ -7,7 +7,7 @@
 const chai = require('chai');
 const {testHelpers} = require('@blockly/dev-tools');
 const {runPlusMinusTestSuite} = require('./test_helpers.mocha');
-const Blockly = require('blockly/node');
+const Blockly = require('blockly');
 const {dartGenerator} = require('blockly/dart');
 const {javascriptGenerator} = require('blockly/javascript');
 const {luaGenerator} = require('blockly/lua');
@@ -195,7 +195,7 @@ suite('Text join block', function () {
         },
       },
     ];
-    runSerializationTestSuite(testCases);
+    runSerializationTestSuite(testCases, Blockly);
   });
 
   suite('Json', function () {
@@ -275,7 +275,7 @@ suite('Text join block', function () {
         },
       },
     ];
-    runSerializationTestSuite(testCases);
+    runSerializationTestSuite(testCases, Blockly);
   });
 
   runPlusMinusTestSuite('text_join', 2, 0, 'ADD', assertTextJoinBlockStructure);

plugins/block-shareable-procedures/test/data_models.mocha.js

@@ -8,7 +8,7 @@ const chai = require('chai');
 const sinon = require('sinon');
 const assert = chai.assert;
 const eventTestHelpers = require('./event_test_helpers');
-const Blockly = require('blockly/node');
+const Blockly = require('blockly');
 const {ObservableProcedureModel} = require('../src/observable_procedure_model');
 const {ObservableParameterModel} = require('../src/observable_parameter_model');
 const {ProcedureCreate} = require('../src/events_procedure_create');

plugins/block-shareable-procedures/test/event_procedure_change_return.mocha.js

@@ -7,7 +7,7 @@
 const chai = require('chai');
 const sinon = require('sinon');
 const assert = chai.assert;
-const Blockly = require('blockly/node');
+const Blockly = require('blockly');
 const eventTestHelpers = require('./event_test_helpers');
 const {testHelpers} = require('@blockly/dev-tools');
 const {ObservableProcedureModel} = require('../src/observable_procedure_model');

plugins/block-shareable-procedures/test/event_procedure_create.mocha.js

@@ -7,7 +7,7 @@
 const chai = require('chai');
 const sinon = require('sinon');
 const assert = chai.assert;
-const Blockly = require('blockly/node');
+const Blockly = require('blockly');
 const {testHelpers} = require('@blockly/dev-tools');
 const eventTestHelpers = require('./event_test_helpers');
 const {ObservableProcedureModel} = require('../src/observable_procedure_model');

plugins/block-shareable-procedures/test/event_procedure_delete.mocha.js

@@ -7,7 +7,7 @@
 const chai = require('chai');
 const sinon = require('sinon');
 const assert = chai.assert;
-const Blockly = require('blockly/node');
+const Blockly = require('blockly');
 const eventTestHelpers = require('./event_test_helpers');
 const {testHelpers} = require('@blockly/dev-tools');
 const {ObservableProcedureModel} = require('../src/observable_procedure_model');

plugins/block-shareable-procedures/test/event_procedure_enable.mocha.js

@@ -7,7 +7,7 @@
 const chai = require('chai');
 const sinon = require('sinon');
 const assert = chai.assert;
-const Blockly = require('blockly/node');
+const Blockly = require('blockly');
 const eventTestHelpers = require('./event_test_helpers');
 const {testHelpers} = require('@blockly/dev-tools');
 const {ObservableProcedureModel} = require('../src/observable_procedure_model');

plugins/block-shareable-procedures/test/event_procedure_parameter_create.mocha.js

@@ -7,7 +7,7 @@
 const chai = require('chai');
 const sinon = require('sinon');
 const assert = chai.assert;
-const Blockly = require('blockly/node');
+const Blockly = require('blockly');
 const eventTestHelpers = require('./event_test_helpers');
 const {testHelpers} = require('@blockly/dev-tools');
 const {ObservableProcedureModel} = require('../src/observable_procedure_model');

plugins/block-shareable-procedures/test/event_procedure_parameter_delete.mocha.js

@@ -7,7 +7,7 @@
 const chai = require('chai');
 const sinon = require('sinon');
 const assert = chai.assert;
-const Blockly = require('blockly/node');
+const Blockly = require('blockly');
 const eventTestHelpers = require('./event_test_helpers');
 const {testHelpers} = require('@blockly/dev-tools');
 const {ObservableProcedureModel} = require('../src/observable_procedure_model');

plugins/block-shareable-procedures/test/event_procedure_parameter_rename.mocha.js

@@ -7,7 +7,7 @@
 const chai = require('chai');
 const sinon = require('sinon');
 const assert = chai.assert;
-const Blockly = require('blockly/node');
+const Blockly = require('blockly');
 const eventTestHelpers = require('./event_test_helpers');
 const {testHelpers} = require('@blockly/dev-tools');
 const {ObservableProcedureModel} = require('../src/observable_procedure_model');

plugins/block-shareable-procedures/test/event_procedure_rename.mocha.js

@@ -7,7 +7,7 @@
 const chai = require('chai');
 const sinon = require('sinon');
 const assert = chai.assert;
-const Blockly = require('blockly/node');
+const Blockly = require('blockly');
 const eventTestHelpers = require('./event_test_helpers');
 const {testHelpers} = require('@blockly/dev-tools');
 const {ObservableProcedureModel} = require('../src/observable_procedure_model');

plugins/block-shareable-procedures/test/procedure_blocks.mocha.js

@@ -4,7 +4,7 @@
  * SPDX-License-Identifier: Apache-2.0
  */
 
-const Blockly = require('blockly/node');
+const Blockly = require('blockly');
 const chai = require('chai');
 const assert = chai.assert;
 const sinon = require('sinon');
@@ -2228,7 +2228,7 @@ suite('Procedures', function () {
       },
     },
   ];
-  testHelpers.runSerializationTestSuite(xmlTestCases, globalThis.clock);
+  testHelpers.runSerializationTestSuite(xmlTestCases, Blockly);
 
   const jsonTestCases = [
     {
@@ -2448,5 +2448,5 @@ suite('Procedures', function () {
       },
     },
   ];
-  testHelpers.runSerializationTestSuite(jsonTestCases, globalThis.clock);
+  testHelpers.runSerializationTestSuite(jsonTestCases, Blockly);
 });

plugins/content-highlight/src/index.ts

@@ -8,7 +8,7 @@
  * @fileoverview A plugin that highlights the content on the workspace.
  */
 
-import * as Blockly from 'blockly';
+import * as Blockly from 'blockly/core';
 
 /**
  * List of events that cause a change in content area size.

plugins/dev-create/templates/block/template/test/block_test.mocha.js

@@ -140,7 +140,7 @@ suite('BlockTemplate', function () {
     },
     // TODO add additional test cases.
   ];
-  runSerializationTestSuite(testCases);
+  runSerializationTestSuite(testCases, Blockly);
 
   // TODO add any other relevant tests
 });

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

@@ -15,6 +15,7 @@ const webpack = require('webpack');
 
 const appDirectory = fs.realpathSync(process.cwd());
 const resolveApp = (relativePath) => path.resolve(appDirectory, relativePath);
+const exists = (relativePath) => fs.existsSync(resolveApp(relativePath));
 
 const packageJson = require(resolveApp('package.json'));
 
@@ -23,26 +24,32 @@ module.exports = (env) => {
   const isDevelopment = mode === 'development';
   const isProduction = mode === 'production';
   const isTest = mode === 'test';
-  const isTypescript = fs.existsSync(resolveApp('tsconfig.json'));
+  const isTypescript = exists('tsconfig.json');
 
   let entry;
   let outputFile;
   let target = 'web';
+  const plugins = [
+    // Use DefinePlugin (https://webpack.js.org/plugins/define-plugin/)
+    // to pass the name of the package being built to the dev-tools
+    // playground (via plugins/dev-tools/src/playground/id.js).  The
+    // "process.env."  prefix is arbitrary: the stringified value
+    // gets substituted directly into the source code of that file
+    // at build time.
+    new webpack.DefinePlugin({
+      'process.env.PACKAGE_NAME': JSON.stringify(packageJson.name),
+    }),
+  ];
+
   if (isProduction) {
     // Production.
-    ['js', 'ts']
-      .filter((ext) => fs.existsSync(resolveApp(`./src/index.${ext}`)))
-      .forEach((ext) => {
-        entry = `./src/index.${ext}`;
-      });
+    if (exists('./src/index.js')) entry = './src/index.js';
+    if (exists('./src/index.ts')) entry = './src/index.ts';
     outputFile = 'index.js';
   } else if (isDevelopment) {
     // Development.
-    ['js', 'ts']
-      .filter((ext) => fs.existsSync(resolveApp(`./test/index.${ext}`)))
-      .forEach((ext) => {
-        entry = `./test/index.${ext}`;
-      });
+    if (exists('./test/index.js')) entry = './test/index.js';
+    if (exists('./test/index.ts')) entry = './test/index.ts';
     outputFile = 'test_bundle.js';
   } else if (isTest) {
     // Test.
@@ -56,16 +63,20 @@ module.exports = (env) => {
       });
     outputFile = '[name].mocha.js';
     target = 'node';
-  }
-
-  // Add 'dist' to the end of the blockly module alias if we have acquired
-  // blockly from git instead of npm.
-  let blocklyAliasSuffix = '';
-  const blocklyDependency =
-    (packageJson.dependencies && packageJson.dependencies['blockly']) ||
-    (packageJson.devDependencies && packageJson.devDependencies['blockly']);
-  if (blocklyDependency && blocklyDependency.indexOf('git://') === 0) {
-    blocklyAliasSuffix = '/dist';
+    // Certain optional plugins wanted by dependencies of blockly
+    // (jsdom want canvas, jsdom depends on ws which wants
+    // bufferutils and utf-8-validate) are loaded via:
+    //
+    // try {/*...*/ = require('package')} catch (e) {/*...*/}
+    //
+    // Webpack tries to satisfy the require even though it's in a
+    // try/catch, and issues a warning if it can't be found.
+    // IgnorePlugin suppresses this.
+    plugins.push(
+      new webpack.IgnorePlugin({
+        resourceRegExp: /^(canvas|bufferutil|utf-8-validate)$/,
+      }),
+    );
   }
 
   return {
@@ -84,12 +95,10 @@ module.exports = (env) => {
       clean: true,
     },
     resolve: {
-      alias: {
-        blockly: resolveApp(`node_modules/blockly${blocklyAliasSuffix}`),
-      },
-      extensions: ['.ts', '.js'].filter(
-        (ext) => isTypescript || !ext.includes('ts'),
-      ),
+      extensions: ['.ts', '.js', '.json', '.wasm'],
+      // Some deps may require node.js core modules.  Tell node.js what
+      // polyfills to use for them when building for non-node.js targets
+      // (Or to ignore them if the fallback is false.)
       fallback: {
         util: false,
       },
@@ -111,20 +120,15 @@ module.exports = (env) => {
     // 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({
-        'process.env.PACKAGE_NAME': JSON.stringify(packageJson.name),
-      }),
-      // canvas should only be required by jsdom if the 'canvas' package is
-      // installed in package.json. Ignoring canvas require errors.
-      isTest &&
-        new webpack.IgnorePlugin({
-          resourceRegExp: /canvas$/,
-        }),
-    ].filter(Boolean),
+    plugins,
     externals: isProduction
       ? {
+          'blockly': {
+            root: 'Blockly',
+            commonjs: 'blockly',
+            commonjs2: 'blockly',
+            amd: 'blockly',
+          },
           'blockly/core': {
             root: 'Blockly',
             commonjs: 'blockly/core',