faucet-pipeline / faucet-pipeline-js

JavaScript asset pipeline
Apache License 2.0
7 stars 6 forks source link

Fix Github Actions #196

Closed moonglum closed 2 years ago

moonglum commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

Okay, summing up:

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 commented 2 years ago

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

moonglum commented 2 years ago

You're welcome 🙇