Issue with upgrade from 1.50.1 to 1.50.2

[masonhugus]

Hi, just reporting that the latest version when msgpack was replaced with msgpackr, we now get this error when trying to run our project. We use the esm module (https://www.npmjs.com/package/esm) to load our project.

Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: node_modules/msgpackr/dist/index.js
require() of ES modules is not supported.

[manast]

On which node version is this?

[lukepolo]

im using v12.20.2 , im assuming @masonhugus is using webpack as well ~

[lukepolo]

Full error

(node:1438) UnhandledPromiseRejectionWarning: Error [ERR_REQUIRE_ESM]: Must use import to load ES Module: /opt/qumulex/app/node_modules/msgpackr/dist/index.js
require() of ES modules is not supported.
require() of /opt/qumulex/app/node_modules/msgpackr/dist/index.js from /opt/qumulex/app/node_modules/bullmq/dist/classes/scripts.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 /opt/qumulex/app/node_modules/msgpackr/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 require (internal/modules/cjs/helpers.js:77:18)
    at Object.<anonymous> (/opt/qumulex/app/node_modules/bullmq/dist/classes/scripts.js:8:20)
    at Module._compile (internal/modules/cjs/loader.js:1156:30)
    at Object.Module._extensions..js (internal/modules/cjs/loader.js:1176:10)
    at Module.load (internal/modules/cjs/loader.js:1000:32)
    at Function.Module._load (internal/modules/cjs/loader.js:899:14)
(node:1438) UnhandledPromiseRejectionWarning: Unhandled promise rejection. This error originated either by throwing inside of an async function without a catch block, or by rejecting a promise which was not handled with .catch(). To terminate the node process on unhandled promise rejection, use the CLI flag `--unhandled-rejections=strict` (see https://nodejs.org/api/cli.html#cli_unhandled_rejections_mode). (rejection id: 1)
(node:1438) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

[masonhugus]

@manast we're using v12.22.6

@lukepolo we do use webpack, but not with the project running bullmq

[manast]

@masonhugus Is it even needed to use ESM on node versions 12+ ? Could you consider not using it anymore, seems to be a deprecated project nowadays (2 year since last update).

[manast]

The irony here is that msgpackr tries to be compatible with both pre node 12 (i.e. no es6 module support) and post 12, still the polyfill (ESM) that tries to solve this problem in a generic way fails when it shouldn't do anything as it is already working!

[lukepolo]

Is 1.50 compatible with 1.49 jobs? Just curious

[manast]

@lukepolo sure. This is just an optimized way to pass args and options to the script, but the data is still stored in the exact same way.

[masonhugus]

@manast yeah, that's something we'll look into doing. This isn't the first time a package has done that, though it was a bit surprising to see a breaking change like this on a patch update.

[lukepolo]

were not using ESM and get this issue, so im not sure what todo at this point. i can try to dig later, but sticking to 1.49 for a while.

[manast]

@manast yeah, that's something we'll look into doing. This isn't the first time a package has done that, though it was a bit surprising to see a breaking change like this on a patch update.

Well, it was not supposed to be a breaking change, in fact before this was merged we got the maintainer of msgpackr to release a change that allowed msgpackr to support node 10 as well. Maybe somebody that knows better can give us a hint on what changes to make in msgpackr so that this issue goes away.

[roggervalf]

Probably, it's related to https://stackoverflow.com/questions/42708484/what-is-the-module-package-json-field-for, right now I'm afk until Sunday, then I can submit a pr into msgpackr to try to fix this issue, or maybe in the midtime someone could help on creating it sooner 😄

[kriszyp]

I responded on @roggervalf 's PR (thank you!), but thought I would look for clarification here too: If I understand correctly, the problem is that there is an attempt to require msgpackr, and node (an older version?) is ignoring the exports directive to use node.cjs, and loading the wrong file (require should load dist/node.cjs not dist/index.js according to the package.json). Perhaps this could be fixed by changing msgpackr's main to point to dist/node.cjs? My only concern would be if there are other bundlers that only support main and incorrectly bundle the unnecessary node code?