update node-fetch to v3

[jimmywarting]

Hi, I'm a core team member of node-fetch, part of our v3 release was to ship it as a ESM-only, so you are no longer able to use require to import it...

Using the latest node-fetch release in your SDK was a tiny bit tricky cuz you still depend on shipping dual esm/cjs support. That meant that it can't use regular import syntax either cuz your import syntax is converted into require() calls. So without making a too breaking change and only demand that the dropbox sdk should be shipped as esm-only then i needed to circumvent the compiler by using a Eval function and force the use of lazy import()

dynamic import works from NodeJS v12.20 in commonjs

this way node-fetch will be lazy loaded only when needed, so bootup should be faster...

Checklist

General Contributing

• [x] Have you read the Code of Conduct and signed the CLA?

Is This a Code Change?

• [x] Dependency update

Validation

• [x] Does npm test pass?
• [x] Does npm run build pass?
• [x] Does npm run lint pass?

[greg-db]

Thanks for putting this together! I'm asking the team to review this.

[jimmywarting]

Thanks for putting this together! I'm asking the team to review this.

Cool 👍

Do you know what would be even cooler? Shipping this SDK as a ESM-only also and ditching cjs/umd builds 😉 (but this is totally up to you to decide, it would probably mean no IE support at all)

[jimmywarting]

just took a closer look at your package.json and notice this: https://github.com/dropbox/dropbox-sdk-js/blob/f1214918142ac51ea0b4181c5c1755f7403d8622/package.json#L62-L64

node-fetch@3 only support 12.20 and above... means you would have to target the same version as well. So this PR would probably mean that this is a breaking change?

[greg-db]

Thanks for the notes!

[codecov[bot]]

Codecov Report

Merging #856 (60f1853) into main (f121491) will decrease coverage by 1.30%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main     #856      +/-   ##
==========================================
- Coverage   65.48%   64.17%   -1.31%     
==========================================
  Files           7        7              
  Lines         788      790       +2     
==========================================
- Hits          516      507       -9     
- Misses        272      283      +11     

Flag	Coverage Δ
integration	?
unit	64.17% <100.00%> (+0.09%)	:arrow_up:

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
src/auth.js	86.66% <100.00%> (-1.40%)	:arrow_down:
src/dropbox.js	92.20% <100.00%> (-3.85%)	:arrow_down:
lib/routes.js	50.09% <0.00%> (-1.18%)	:arrow_down:

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update f121491...60f1853. Read the comment docs.

[rogebrd]

Hello Jimmy, looking to get this merged in but it looks like some of the tests are failing.

I think for the time being at least, we want to ship CJS/UMD in addition to ESM due to additional compatibility since we are trying to reach the largest number of consumers as we can.

With that in mind, I think I would also request we modify this to not bump the required version and keep the code such that it can run with both the 2 and 3 major version of node-fetch.

Let me know if this all makes sense and you are able to make the changes! If not, I can take of the diff and go from there

[jimmywarting]

Best you take the diff and go from there

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[jimmywarting]

Now that fetch is built into NodeJS itself then i think you should be doing

const { fetch } = globalThis

if it isn't avalible then user can polyfill it themself with whatever fetch compatible package they choose wether it come from node-fetch, undici nodejs deno or whatever.

[greg-db]

@jimmywarting Thanks for the information!