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

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

Is This a Code Change?

• Dependency update

Validation

• Does npm test pass?
• Does npm run build pass?
• 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:

dropbox-sdk-js/package.json

Lines 62 to 64 in f121491

	"engines": {
	"node": ">=0.10.3"
	},

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]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.17%. Comparing base (f121491) to head (60f1853).
Report is 24 commits behind head on main.

Additional details and impacted files

@@            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%)	⬆️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

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