node-soap icon indicating copy to clipboard operation
node-soap copied to clipboard

Bump `strip-bom` and `get-stream`

Open smokhov opened this issue 3 weeks ago • 8 comments

These dependencies are now compatible with node-soap.

  • Bump strip-bom from 3.0.0 to 5.0.0
  • Bump get-stream from 6.0.1 to 9.0.1

Helps #1281

smokhov avatar Nov 30 '25 02:11 smokhov

I need to think about it. In general, I am trying to keep support for older Node.js as there are many people that still use unsupported node. Soap is specific stuff that is around for ages and likely used on very old environments.

w666 avatar Dec 03 '25 09:12 w666

I need to think about it. In general, I am trying to keep support for older Node.js as there are many people that still use unsupported node. Soap is specific stuff that is around for ages and likely used on very old environments.

Our min node is already 20 judging by other deps' min node requirement of 20. These ones require 18 minimum, but as I said the codebase is already past that to 20.

smokhov avatar Dec 03 '25 17:12 smokhov

I think I was testing it on Node 16, but I will double check that.

w666 avatar Dec 03 '25 19:12 w666

yes, current min version is node 20. But update to latest get-stream bumps it to 22, this is bit extreme for project like this. I think we should keep it compatible with 20, at least while this is still supported.

w666 avatar Dec 06 '25 19:12 w666

yes, current min version is node 20. But update to latest get-stream bumps it to 22, this is bit extreme for project like this. I think we should keep it compatible with 20, at least while this is still supported.

This patch for get-stream bumps it to "node": ">=18" and strip-bom to "node": ">=12"

smokhov avatar Dec 06 '25 20:12 smokhov

As per https://github.com/vpulim/node-soap/pull/1399 it does not work on node 18. I tested on all latest versions from 14 up to 24.

w666 avatar Dec 07 '25 08:12 w666

The problem with the change in this PR it does not work on node 20, it will require some changes to convert it to esm.

Exception during run: Error [ERR_REQUIRE_ESM]: require() of ES Module 

though it works as is on node 22 and 24. I will look into it why as I am not aware of the changes on node 22 that allow esm modules to be used in commonjs.

w666 avatar Dec 07 '25 08:12 w666

The problem with the change in this PR it does not work on node 20, it will require some changes to convert it to esm.

Exception during run: Error [ERR_REQUIRE_ESM]: require() of ES Module 

though it works as is on node 22 and 24. I will look into it why as I am not aware of the changes on node 22 that allow esm modules to be used in commonjs.

Oh, I see. Then I think it's misrepresented in in this deps, I guess. I agree this PR needs to be held, 20 is still the supported one. Moving to 22 is too drastic.

smokhov avatar Dec 08 '25 18:12 smokhov