Bump strip-bom and get-stream

[smokhov]

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
• Bump min node version to 20.19.0
• Don't fast-fail the test matrix

Helps #1281

[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.

[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.

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.

[w666]

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

[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.

[smokhov]

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"

[w666]

As per #1399 it does not work on node 18. I tested on all latest versions from 14 up to 24.

[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.

[smokhov]

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.

[w666]

I improved pipeline so it now runs against node 20, 22 and 24. You can rebase this MR and see if it fails against 20. Hopefully it should find issue with compatibility in this MR.

[w666]

interesting, it worked, I will need to investigate this.

[smokhov]

@w666 -- all 3 checks are successful... what's ... wrong?

[smokhov]

@w666 -- GitHub's default Node 20 is 20.20.0, where it works; node 20.9.0 the package is configured for, fails. So there were changes between Node 20.9.x to 20.20.x that fixed the Error [ERR_REQUIRE_ESM]: require() of ES Module error. I have added temporarily 20.9.0 to the matrix and disabled fail-fast to show that. I have not tested other versions between 20.9.0 and 20.20.0 where the change has taken place fixing the error. Not sure what you want to do about it.

[smokhov]

@w666 -- GitHub's default Node 20 is 20.20.0, where it works; node 20.9.0 the package is configured for, fails. So there were changes between Node 20.9.x to 20.20.x that fixed the Error [ERR_REQUIRE_ESM]: require() of ES Module error. I have added temporarily 20.9.0 to the matrix and disabled fail-fast to show that. I have not tested other versions between 20.9.0 and 20.20.0 where the change has taken place fixing the error. Not sure what you want to do about it.

So I did quick test with expanding the matrix and as of this writing, Node 20.[9-18].0 fail and Node 20.[19-20].0 succeed in passing the tests... What gives...

[w666]

@smokhov that is awesome, thanks for this. I guess we can release this feature then, just need to update package.json with the minimal node version.

I guess we can do this soonish, people are migrating very slow, se we should give them some time.

[w666]

Since there was recent maintenance release with up-to-date deps I think this can be merged.