Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Node.js 16 currently no longer working becauseof worker-timers #1802

Closed
Apollon77 opened this issue Feb 28, 2024 · 8 comments · Fixed by #1813 or fvilarinho/akamai-siem-connector#35
Labels

Comments

@Apollon77
Copy link

MQTTjs Version

several?

Broker

n/a

Environment

NodeJS

Description

Currently mqtt packsge is no longer supporting Node.js 16 because of chrisguttandin/worker-timers-broker#282 ... This is mainly FYI because your package.json states that node.js 16 is supported

Minimal Reproduction

n/a

Debug logs

n/a

@Apollon77 Apollon77 added the bug label Feb 28, 2024
@robertsLando
Copy link
Member

@Apollon77 Thanks for spotting this issue, in the meanwhile I could use override to fix this, what do you think?

@Apollon77
Copy link
Author

In theory yes ...

@chrisguttandin
Copy link
Contributor

Hi @robertsLando, I'm the author of the package mentioned above. The odd thing to me is that it doesn't support Node.js itself. However it has a dependency which supports Node.js and the browser. The engines property of that dependency is what causes the problem, although it should be totally irrelevant in the end.

Here are two related issues with a bit more context:
chrisguttandin/worker-timers-broker#282chrisguttandin/fast-unique-numbers#482

@robertsLando
Copy link
Member

robertsLando commented Feb 28, 2024

I accept suggestions... IMO available solutions right now are:

  • Use override on my side to strick worker-timers-broker to 6.1.1 (the version right before the fast-unique-numbers@9.0.0 bump
  • Make a new major 6.0.0 on that drops v16
  • @chrisguttandin Create a new patch release of worker-timers-broker with a lower version of fast-unique-numbers (the one before 9.0.0) and then create a major release of it that drops v16 support.
  • ??

@robertsLando
Copy link
Member

robertsLando commented Feb 29, 2024

@chrisguttandin I'm mostly for the solution 3 not because I don't need to to anything but mostly because I think that it makes more sense as other users may have the same issue and while I understand your point of view it's wrong from a semantic versioning side to have a minor/patch version causing such issue

Anyway if you tell me that you don't want to do that I will go for one of the others two on my side

@chrisguttandin
Copy link
Contributor

For a brief moment I thought I could maybe add a engines property to worker-timers to override the one in fast-unique-numbers and to indicate that engines don't matter in this context. But if that would work the engine property in mqtt should do that already.

I'm not sure, maybe I should drop the engines property entirely. But for now I'm finally convinced that option 3 is the most pragmatic. It will probably also confuse a few people but it's at least easier to ignore if you're not impacted.

I'm thinking about making all my packages that use a Web Worker to be compliant with worker_threads anyway. It wouldn't bring any technical advantage for worker-timers but it would at least make the usage a lot easier. It would for example allow you to remove the extra checks from your code base.

But I'll leave that for another day. :-)

@chrisguttandin
Copy link
Contributor

@robertsLando I've created #1813 to update worker-timers to the latest version.

@robertsLando
Copy link
Member

I'm thinking about making all my packages that use a Web Worker to be compliant with worker_threads anyway. It wouldn't bring any technical advantage for worker-timers but it would at least make the usage a lot easier. It would for example allow you to remove the extra checks from your code base.

That would be great! Let me know if you do that so I can update my code accordingly 👍🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants