On Hold: Update Axios to latest version: 1.6.2

[janpaepke]

UPDATE It seems this PR fails with node 6.14. See discussion below for more info.

---

This PR updates the Axios dependency to 1.6.2, since the version currently in use (0.27.2) contains a reported vulnerability (Cross-Site Request Forgery).

Even though this is actually of no consequence in the context of the usage of this library, this results in warning messages for all consumers, who will have to then research if a risk exists. This resolves: https://github.com/mollie/mollie-api-node/issues/337

The update is technically pretty straight forward, since there appears to be no breaking change in behaviour within the scope of this library. The only changes to the library are type related.

Most of the work spent on this PR was getting the tests to work. Since changes to the tests are obviously a delicate matter, I broke the updates down into individual commits for better reviewability:

1. upgrade axios and update types: This includes the version update of axios and the only necessary changes to get the build to compile. As mentioned before, these are only type related.
2. upgrade jest and fix uncontroversial tests: This contains three fairly uncontroversial changes:
   • Update jest: One major difference in axios is that it points to an es6 file as the main entry point. Since tests are run on dist files, only cjs is supported and the old jest version was unable to properly resolve the axios cjs lib source. This is fixed in jest 29. (actually I think since 28).
   • Snapshot updates: The snapshot format in the jest 29 does not include the prototype name by default. There are no snapshot changes other than the removal of the prototype names.
   • Axios http adapter definition: The http adapter is not imported anymore, but referenced by string.
3. fixed tests, which require some explanation:
   • Type mismatch: Jest no longer exposes a global fail method, even though DefinitelyTyped still thinks so. This can be easily fixed by importing the fail method from node:assert, which has the same behavior in our context.
   • Promise OR done(): Jest 29 forces users to either use done OR return a promise. That's an easy fix!
4. fix axios-timing related tests: This issue was quite time consuming to identify, but it seems the new axios version does not play nice with jest.useFakeTimers(). I suspect the reason to be a changed logic for their internal request timeout management. As a consequence the initial request would not be processed, when simply awaiting tick (). What I did find to work was using await jest.advanceTimersByTimeAsync(0); instead, which should have the same effect. Additionally this is how we awaited subsequent requests: jest.advanceTimersByTime(2e3); await tick(); The only method I found to work reliably now was this: await tick(); await jest.advanceTimersByTimeAsync(2e3); Swapping the order and making the timer async should still not change the intention of this test.

In conclusion, this should be an appropriate upgrade with no breaking changes for the lib. The only part that might deserve a closer look are the changes describe in #4 above.

[janpaepke]

All right, so in contrast to my naive initial conclusion, this seems to be a less trivial update.

The reason is that this sdk currently supports node 6, which is dropped in jest 25.

I investigated further, if there is a way to get jest 24 to understand how to resolve the correct axios cjs files and got it to work by updating the jest.config.js like this:

function createProject(displayName, testRegex, rest) {
  return Object.assign({}, rest, {
    …
    moduleNameMapper: {
      axios: 'axios/dist/node/axios.cjs',
    },
    …
  });
}

While this does make jest resolve axios correctly, it still breaks, because axios.cjs seems to contain the spread operator, which was only supported starting node 8.6.

So not only is axios apparently incompatible with node 6 (there is no minimum node version marked in the package), but even if it was, it's likely that any user actually still using node 6 would run into the import issue, which makes this a bad solution.

The way I see it there are three possible ways forward:

1. rip out axios and replace it or write a custom http client implementation.
2. try to include axios in the output bundle rather than making it a dependency, transpiling accordingly.
3. drop support for node 6 and use node 14 as minimum supported version instead.

Personally I would recommend 3, which would make this a breaking change. Both 1 AND 2 require more work, where the effort is most likely unjustified to support an ancient node version. Node 14 was released almost 4 years ago and is currently used by most libraries (including jest 29) as the minimum supported version. I have confirmed this PR does work on node 14.

To decide, it would be interesting to get some usage stats from the MollieAPI maintainers. For now this PR is on hold, until we decide on a path forward.

[DHFW]

I would vote for dropping support for the very old NodeJS versions (I am on Node 18). I really want my payment provider to not have CVE's... Release a version 4 so that it is clear this is a major upgrade.

[janpaepke]

As an additional side note: You'd think another option would be to "only" increase the node requirement to node 8.6 to support the spread operator.

Well I tried that, but unfortunately the axios.cjs dist also includes an async iterator:

Support for this was only introduced in node 10.

Using node 10 and the jest "fix" above, axios-mock-adapter breaks. Same for node 12. This is likely because it fails to resolve its peer deependency for the same reason we require the jest config update.

Maybe there's a way to get it to work with node 10, but is the effort worth it? I mean if we're doing a breaking change anyway, why not go 14?

Looking forward to some maintainer insights on this.

best Jan

[fjbender]

@maria-swierblewska Can you look into the numbers a bit and then come up with a recommendation around dropping version support?

[maria-swierblewska]

Hello @janpaepke I've done some analysis on past year's data on Node versions used versus the volume of payments they support and it is clear that the support should be maintained for v14 and higher (v14 supports almost 30% of all volume). In the past year all versions under v14 account for 2.94% of all payments volume so indeed we think there is no need to support the versions below 14. Let me know if you have any other questions on this!

[janpaepke]

@maria-swierblewska thank you for your info.

@fjbender We could either prepare a separate branch with the v14 update ONLY (package json, yaml etc.) to update everything and then rebase this one, or keep working in this branch. How do you propose we proceed?

[maria-swierblewska]

@janpaepke for the sake of transparency we'd suggest to proceed with a separate branch with the v14 update and rebasing this one

[janpaepke]

Added the node update here: https://github.com/mollie/mollie-api-node/pull/341

This should be pretty straight forward.

[DHFW]

Thanks for all the work! When will this be released? Edit: nvm, I see I need to install the 4.0.0 beta.