fix(dev-scripts)!: Fixes, refactoring and simplification of `webpack.config.js` and `'blockly'` imports

[cpcallen]

The basics

• [x] I validated my changes

The details

Resolves

Part of google/blockly#7449.

Proposed Changes

• Remove support acquiring Blockly through git. This completes the revert of PR #335. See BREAKING CHANGE below for more details.

• Don't use alias when resolving blockly. PR #226 added 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.

• Exclude Blockly from plugin bundles. This fixes bloat caused by some plugins importing all of 'blockly' (instead of just 'blockly/core'), resulting in webpack including a copy of blockly in the bundled plugin because only the subpackage entrypoints were listed in the externals stanza of webpack.config.js. This will also avoid certain problems that might occur due to apps using such bundles 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'.

• Introduce an exists() function for readability.

• Ignore more jsdom subdependencies: add bufferutils and utf-8-validate to the IgnorePlugin config when building tests. These are optional dependencies of wd, which is itself a dependency of jsdom. Also refactor how the plugins config is generated to improve readability.

• Simplify resolve.extensions. There doesn't appear to be any harm in including '.ts' in resolve.extensions even for pure-JS plugins, but it is necessary to include it for 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.

• Add various comments to help future readers.

• Update several plugin's tests/*.mocha.[jt]s files to import '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.

Reason for Changes

• Ensure plugins will continue to build after google/blocky#7822 is merged and published.
• Improve readability of webpack.config.js.

Test Coverage

The build, test and start scripts for each plugin were tested against both Blockly v10.4.2 and Blockly v11.0.0.beta-3 + google/blockly#7822.

Additional Information

BREAKING CHANGE: This PR removes the support that was added in PR #335 for acquiring 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 blockly-samples—or wait for blockly to become a monorepo so both changes can be made in the same PR!

[cpcallen]

Note to reviewer: it may be helpful to review this PR commit-by-commit.

[maribethb]

I haven't reviewed this yet, but I will note that doing a breaking change in dev-tools/dev-scripts will cause every other plugin to also get a major version bump. Since we don't "need" this until blockly v11, which will independently also cause a major version bump in every plugin, I would prefer to bundle those together and therefore submit this to the v11-specific branch in samples.

[cpcallen]

[Edited for clarity.]

I will note that doing a breaking change in dev-tools/dev-scripts will cause every other plugin to also get a major version bump.

That is surprising. This change to dev-scripts should not have any effect on the contents (and thus the backward-compatibility and major version number) of any of the published plugin packages, since currently none of them depends on a git:// URL.

In the general case, while it's possible that any change to any kind of dependency could result in a breaking change to the dependent package, it's not automatic, and breaking changes to dev dependencies are even less likely to cause one than breaking changes to non-dev dependencies.

Is this a result of our lerna config / publish scripts?

[cpcallen]

[Comment moved from #2229 as it looks like this one will probably be the one that gets merged.]

CI is failing here and in #2229 due to multiple similar test failures in plugins/block-dynamic-connection, e.g.:

  1) If block
       Serialization
         append block
           one if one else - old serialization:
     TypeError: Failed to execute 'appendChild' on 'Node': parameter 1 is not of type 'Node'.
      at exports.convert (webpack-internal:///../dev-tools/node_modules/jsdom/lib/jsdom/living/generated/Node.js:25:9)
      at Element.appendChild (webpack-internal:///../dev-tools/node_modules/jsdom/lib/jsdom/living/generated/Node.js:404:26)
      at blockToDom$$module$build$src$core$xml (webpack-internal:///../dev-tools/node_modules/blockly/blockly_compressed.js:163:393)
      at blockToDomWithXY$$module$build$src$core$xml (webpack-internal:///../dev-tools/node_modules/blockly/blockly_compressed.js:161:195)
      at new BlockCreate$$module$build$src$core$events$events_block_create (webpack-internal:///../dev-tools/node_modules/blockly/blockly_compressed.js:610:462)
      at domToBlockInternal$$module$build$src$core$xml (webpack-internal:///../dev-tools/node_modules/blockly/blockly_compressed.js:180:405)
      at $.domToBlock$$module$build$src$core$xml [as domToBlock] (webpack-internal:///../dev-tools/node_modules/blockly/blockly_compressed.js:178:81)
      at Context.eval (webpack-internal:///../dev-tools/src/block_test_helpers.mocha.js:191:63)

These errors have the same root cause the test failures in #1452: multiple copies of jsdom.

In that case the issue was that each of the tests/*.mocha.js files get webpacked with their own separate copy of blockly + jsdom, and due to how Blockly` handled polyfilling XML DOM functions on Node this resulted in some of the copies of Blockly not being able to do (de)serialisation.

In this case the issue appears to be that, due to the removal of the alias clause from webpack.config.js, each of the tests/*.mocha.js bundles ends up getting at least two separate copies of blockly + jsdom: one from plugins/dev-tools/node_modules/ and one from plugins/block-dynamic-connection/node_modules/. This then results in XML DOM objects created by runSerializationTestSuite (using it's copy of jsdom) failing type checks done by the copy of jsdom used by the instance of blockly that is smuggled in to that runSerializationTestSuite via this.workspace.

Additional clarification:

• In the previous case the multiple copies of jsdom were being created by a single copy of jsdom being ingested multiple times by webpack; in the current case the cause is multiple separate copies of jsdom in different node_modules directories. (There were multiple copies before, too, but the alias entry for blockly in webpack.config.js caused only one to be used.)

• This problem applies to any blockly-samples plugin using runSerializationTestSuite; blockly-dynamic-connection is just the first of these to fail when running npm test at the top level.)

• The problem probably only affects plugins in blockly-samples. Specifically, failure depends on the @blockly/dev-tools package being installed with a copy of blockly (and thus jsdom) in its node_modules directory, which should only happen if it is checked out for development rather than being installed as a package from the npmjs repository.

Possible fixes:

1. Pass Blockly into runSerializationTestSuite as an argument, instead of importing it.
2. Figure out how to ensure that there is only one copy of (blockly and) jsdom that is shared between the plugins.
   • Using npm link to link each of the plugins to a local copy of blockly/dist achieves this, but is not a suitable solution for testing in CI at the present time. This is, however, why this failure was not noticed locally when this PR was being prepared.
   • Such a change would seem like good hygiene, in any case, though it raises a question about what should happen if different plugins want to test with different versions of Blockly. Probably only a temporary concern, however, as in a monorepo we would always test against the current version of Blockly.
3. Figure out how to get webpack to include only one copy of jsdom in the bundle for each .mocha.js file.
   • In Blockly v11 we can't alias blockly as this causes package.json to be ignored, but it might be possible to alias jsdom or find some other way to ensure that only one copy is loaded.
   • Unfortunately this means that these tests become dependent on being build with webpack, since running them directly in node.js will still obey the normal node package resolution rules.
4. Fix Blockly so it no longer has global state.
5. Make jsdom an optional peer dependency of Blockly (I think…)
6. Remove jsdom as a dependency of Blockly and force node.js users to inject it manually. (This might create problems for node.js users who want to use packages that depend on Blockly which don't provide a mechanism to inject it there, however.)
7. Make Blockly's use of jsdom entirely via global variables, so all copies would share a single instance.

[cpcallen]

Even after modifying runSerializationTestSuite to work around the problem caused by multiple copies of jsdom being included in each test bundle, one additional test, in plugins/typed-variable-modal, is still failing:

  1) TypedVariableModal
       show()
         Elements focused:
     TypeError: Cannot read properties of undefined (reading 'appendChild')
      at TypedVariableModal.widgetCreate_ (webpack-internal:///../modal/src/index.js:159:15)
      at TypedVariableModal.show (webpack-internal:///../modal/src/index.js:117:10)
      at Context.eval (webpack-internal:///./test/typed_variable_modal_test.mocha.js:93:26)

The root cause is the same, but this time it's the fact that there are two copies of the blockly npm package included in the test bundle: one for typed-variable-modal and a second one for modal. The former is used for the Blockly.inject() call and has containerDiv (in core/widgetdiv.ts) set, while the latter is used when modal's Modal.prototoype.widgetCreate_ method calls Blockly.WidgetDiv.getDiv, which finds that containerDiv in that copy of Blockly is still undefined and throws the error seen above.

After discussing this with the core team it has been decided to "solve" this problem by disabling this particular test for the time being, since we expect that moving to the proposed new, npm-workspace-based arrangement will solve this problem (and also the one that necessitated the previous change to runSerializationTestSuite).

[cpcallen]

Ok, @maribethb: this and #2229 (same changes, but on v11 branch) are now ready for review.