Closed Vhndaree closed 10 months ago
Hi @marck283 Thanks, that is a great idea but
node-fetch
changed to ES modules by default from v3 so if we want to stick with the similar pattern of require() then it is not possible at the moment. So, I am sticking with v2.7.
I understand your point but consider that solving the MaxListenersExceededWarning warning will lead to less memory usage. As shown in these test runs (taken from here):
without the patch:
memoryUsage.rss: 35.97 Mb ⇒ 96.75 Mb (Diff: 168.99%)
memoryUsage.heapTotal: 7.11 Mb ⇒ 49.87 Mb (Diff: 601.10%)
memoryUsage.heapUsed: 5.23 Mb ⇒ 29.92 Mb (Diff: 471.66%)
memoryUsage.external: 1.04 Mb ⇒ 16.76 Mb (Diff: 1512.99%)
memoryUsage.arrayBuffers: 0.03 Mb ⇒ 15.68 Mb (Diff: 50570.52%)
with the patch:
memoryUsage.rss: 35.87 Mb ⇒ 47.92 Mb (Diff: 33.61%)
memoryUsage.heapTotal: 7.11 Mb ⇒ 7.62 Mb (Diff: 7.14%)
memoryUsage.heapUsed: 5.22 Mb ⇒ 6.86 Mb (Diff: 31.51%)
memoryUsage.external: 1.04 Mb ⇒ 8.10 Mb (Diff: 679.89%)
memoryUsage.arrayBuffers: 0.03 Mb ⇒ 0.21 Mb (Diff: 592.11%)
the memory usage cuts to half or more in all the tested cases.
When it comes to the require() pattern, Node.js users that run on version 13.2 and above can import the module by using the dynamic import instruction. This is why I think you should change the require() instructions to dynamic imports, so users that run on versions of Node.js below 13.2 should be able to import this module using the standard require(). Furthermore, by using the dynamic import instructions you can upgrade the node-fetch package this module runs on to its latest version. To allow the usage of the dynamic imports, however, you will have to change something in the files containing the TypeScript compilation settings. For example, the file tsconfig.json should be modified as this (changing the file extension back to JSON): tsconfig.txt
Fixes https://github.com/mapbox/node-pre-gyp/issues/697