Fix Github Actions

[moonglum]

Updated the Github Actions, because they were using ancient Node versions. After that, I noticed that some tests are still failing. Some dependencies have changed their output. To decrease duplicated work, I updated all dependencies before doing adjustments. Everything is up to date now except @rollup/plugin-node-resolve, which breaks everything in a non-obvious and non-documented way. So I moved that to "later".

[moonglum]

All IIFEs are now )() instead of ())

[FND]

interesting, I wonder why (but don't have the heart to research it now)

[moonglum]

I found the PR that introduced the change: rollup/rollup#4215
You can see it in the blame view here: https://github.com/rollup/rollup/blame/master/test/form/samples/mjs/_expected/iife.js

I don't really see a reason why though.

[moonglum]

What this PR introduces is the output.generatedCode option which you can use to configure what JS syntax should be used for code generated by rollup (not the code you wrote yourself, but the code around it).

If I add the following to lib/bundle/config.js:

cfg.output = {
  generatedCode: "es2015"
}

Then, the resulting code of test_bundle_customization for IIFE will for example look like this:

--- a/./expected_iife.js
+++ b/./dist/bundle_iife.js
@@ -1,10 +1,5 @@
-var MYLIB = (function () {
-'use strict';
-
-var index = _ => {
+const index = _ => {
        console.log("lipsum");
 };

-return index;
-
-})();
+export { index as default };

[FND]

Oh, good find - I'd been wondering about modern syntax there for a while; we should keep that in mind for v3.0?

[moonglum]

Yes, I think we should look at it for 3.0 👍 Not sure if we need an additional option there, or if we determine it from the browserslist. But definitely worth consideration.

[FND]

Thanks for doing this, your comments were very helpful too. Looking good so far!

I might look into the remaining issues myself (plugin-node-resolve, exclude), but don't hold your breath...

[FND]

interesting, I wonder why (but don't have the heart to research it now)

[moonglum]

So for me this looks like they simplified the generation of CommonJS Modules. Instead of injecting the code to generate the module, instead, they generate the module at built time.

[moonglum]

When updating rollup/plugin-node-resolve from 11.2.1 (which works fine) to 13.0.0 (12 does not exist, it seems to be yanked), we get the following error when compiling:

TypeError: this.resolve is not a function
    at Object.resolveId (node_modules/@rollup/plugin-node-resolve/dist/cjs/index.js:1080:45)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
    at async resolveId (node_modules/rollup/dist/shared/rollup.js:21767:26)
    at async ModuleLoader.resolveId (node_modules/rollup/dist/shared/rollup.js:22067:19)
    at async /home/vagrant/Code/faucet-pipeline-js/node_modules/rollup/dist/shared/rollup.js:22353:42 {
  code: 'PLUGIN_ERROR',
  hook: 'resolveId',
  plugin: 'diskless',
  watchFiles: [
    'diskless:entry_point_1ae80c752ee1690c96fe83ddd2738d49.js'
  ]
}

When updating to 13.1.3 (the latest at point of writing), we get this error instead:

TypeError: Cannot read property 'custom' of undefined
    at Object.resolveId (node_modules/@rollup/plugin-node-resolve/dist/cjs/index.js:1165:83)
    at Object.resolveId (lib/bundle/diskless.js:18:20)
    at /home/vagrant/Code/faucet-pipeline-js/node_modules/rollup/dist/shared/rollup.js:22795:37 {
  code: 'PLUGIN_ERROR',
  hook: 'resolveId',
  plugin: 'diskless',
  watchFiles: [
    'diskless:entry_point_2b1a97035d3d3193e06c2ebe612e7348.js'
  ]
}

[moonglum]

Okay, summing up:

• When updating node-resolve to 13.X, the CLI test suite as well as the "bundling" unit test pass. The "virtual" unit tests both fail with "Cannot read property 'custom' of undefined".
• Due to the backtrace, as well as the locallity of test failure, I would assume that this is a failure that occurs from the combination of node-resolve and our "diskless" plugin.
• The changelog for the version points out a breaking change, but I don't see how it could lead to the error.

As we say in German, right now "bin ich mit meinem Latein am Ende". I don't know enough about both the diskless as well as the node-resolve Plugin. My vote would be:
Let's merge this PR, it is a signficant improvement (updating all our packages to their latest version except one). Then, let's release it (probably as a minor, given that TypeScript and Babel both allow new features?). The issue with node-resolve should be treated separately in its own PR, but this way it doesn't block the release.

What do you think, @FND?

[FND]

added minor tweaks to CI files (file extensions, aesthetics, actions' versions) - ran out of time for now, so will do an explanatory squash commit and release some other time

[FND]

squashed as f3230c7 and released as v2.1.8 - thank you very much!

[moonglum]

You're welcome 🙇