aclap-dev / vdhcoapp

Companion application for Video DownloadHelper browser add-on
GNU General Public License v2.0
1.72k stars 280 forks source link

Upgrade to node 16, fixing ESM loading issue #149

Closed kiesel closed 1 year ago

kiesel commented 1 year ago

On arch, when running the companion application on commandline, it immediately fails with the following stacktrace:

$ vdhcoapp
pkg/prelude/bootstrap.js:1359
      throw error;
      ^

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /snapshot/vdhcoapp-1.6.3/node_modules/flatted/cjs/index.js
require() of ES modules is not supported.
require() of /snapshot/vdhcoapp-1.6.3/node_modules/flatted/cjs/index.js from /snapshot/vdhcoapp-1.6.3/node_modules/log4js/lib/LoggingEvent.js is an ES module file as it is a .js file whose nearest parent package.json contains "type": "module" which defines all .js files in that package scope as ES modules.
Instead rename index.js to end in .cjs, change the requiring code to use import(), or remove "type": "module" from /snapshot/vdhcoapp-1.6.3/node_modules/flatted/package.json.

    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1172:13)
    at Module.load (internal/modules/cjs/loader.js:1000:32)
    at Function.Module._load (internal/modules/cjs/loader.js:899:14)
    at Module.require (internal/modules/cjs/loader.js:1042:19)
    at Module.require (pkg/prelude/bootstrap.js:1338:31)
    at require (internal/modules/cjs/helpers.js:77:18)
    at Object.<anonymous> (/snapshot/vdhcoapp-1.6.3/node_modules/log4js/lib/LoggingEvent.js:4:17)
    at Module._compile (pkg/prelude/bootstrap.js:1433:22)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1176:10)
    at Module.load (internal/modules/cjs/loader.js:1000:32) {
  code: 'ERR_REQUIRE_ESM'
}

This happens as dependency log4js depends on the fatted package, which provides npm modules for commonJS and ECMAScript module ("ESM") - but the bundled runtime (node 12) does not support any of this.

This MR bumps the dependency to node 16, and therefore requires a newer pkg. Since pkg-fetch is actually just a transitive dependency, I've also dropped it from package.json.

In a subsequent step one should check all dependencies for updates, since there are some issues mentioned by npm audit.

paulrouget commented 1 year ago

We are migrating to Node 18. Which will superseed this PR.

Thanks for looking into it though!

xiota commented 1 year ago

When will the Node 18 migration expected to be done?

paulrouget commented 1 year ago

When will the Node 18 migration expected to be done?

It's actively being worked on. So "soon" :)