-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Bump strip-bom and get-stream
#1394
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
base: master
Are you sure you want to change the base?
Conversation
Bumps [strip-bom](https://github.com/sindresorhus/strip-bom) from 3.0.0 to 5.0.0. - [Release notes](https://github.com/sindresorhus/strip-bom/releases) - [Commits](sindresorhus/strip-bom@v3.0.0...v5.0.0) --- updated-dependencies: - dependency-name: strip-bom dependency-version: 5.0.0 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
Bumps [get-stream](https://github.com/sindresorhus/get-stream) from 6.0.1 to 9.0.1. - [Release notes](https://github.com/sindresorhus/get-stream/releases) - [Commits](sindresorhus/get-stream@v6.0.1...v9.0.1) --- updated-dependencies: - dependency-name: get-stream dependency-version: 9.0.1 dependency-type: direct:production update-type: version-update:semver-major ... Signed-off-by: dependabot[bot] <support@github.com>
|
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. |
|
I think I was testing it on Node 16, but I will double check that. |
|
yes, current min version is node 20. But update to latest |
This patch for |
|
As per #1399 it does not work on node 18. I tested on all latest versions from 14 up to 24. |
|
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. 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. |
|
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. |
|
interesting, it worked, I will need to investigate this. |
|
@w666 -- all 3 checks are successful... what's ... wrong? |
These dependencies are now compatible with
node-soap.Helps #1281