Fallback to node-fetch in browser

[defko]

🐛 Bug Report

When 'fetch' is not available in the browser the request fallback to node-fetch instead of sending an "XMLHttpRequest" or "ActiveXObject" request.

//Code snippet from `lib/request.js`
if (fetchApi) {
    // use fetch api
    return requestWithFetch(options, url, payload, callback)
}

if (typeof XMLHttpRequest === 'function' || typeof ActiveXObject === 'function') {
    // use xml http request
    return requestWithXmlHttpRequest(options, url, payload, callback)
}

This causes an issue as node-fetch sets its own user-agent

if (!headers.has('user-agent')) {
 headers.set('user-agent', 'node-fetch/1.0 (+https://github.com/bitinn/node-fetch)');
}

and the issue is: Refused to set unsafe header "user-agent"

To Reproduce

Just disable fetch. Personally I face this issue meanwhile running Cypress test which currently doesn't support fetch.

Expected behavior

I expect the node-fetch fallback only happens when we are on node/deno environment but not in the browser.

Your Environment

Cypress test environment

[adrai]

Strange, there's also a dedicated test to test non-fetch environments: npm run test:xmlhttpreq Can you create a reproducable example?

[defko]

Okay, I see where the issue is coming from. I am using webpack and it supports CommonJS so require is available in that context, therefore it is able to export node-fetch. I assume you don't have such a setup in your test env and that is why you don't experience with this problem.

//getFetch.cjs
if (typeof require !== 'undefined') {
  var f = fetchApi || require('node-fetch')
  if (f.default) f = f.default
  exports.default = f
  module.exports = exports.default
}

Do you still need an example or is this description provides enough context?

[adrai]

Any idea how this could be fixed?

[defko]

Could we check if it is node env maybe

const isNode =
  typeof process !== 'undefined' &&
  process.versions != null &&
  process.versions.node != null;

But as I see you support Deno as well, so in that case I don't know if it will work maybe better to check if it is NOT a browser, like

const isBrowser = 
  typeof window !== 'undefined' && 
  typeof window.document !== 'undefined';

I did not check this, but I suppose the tests should fail if it is not the right check.

[adrai]

Can you test this in your environment? just modify it directly in the node_modules... I think this will not work, because when running webpack this is node that runs it...

[defko]

These solutions could work

const isNode =
  typeof process !== 'undefined' &&
  process.versions != null &&
  process.versions.node != null;

if (isNode && !fetchApi && fetchNode) fetchApi = fetchNode.default || fetchNode

or

const isBrowser =
  typeof window !== 'undefined' &&
  typeof window.document !== 'undefined';

if (!isBrowser && !fetchApi && fetchNode) fetchApi = fetchNode.default || fetchNode

---

Only now the XHR request fall into the error case as x.statusText === "OK" in

//lib/request.js
callback(x.statusText, { status: x.status, data: x.responseText })

and it is handled as err by

//index.js
    this.options.request(this.options, url, undefined, (err, res) => {
      if (res && res.status >= 500 && res.status < 600) return callback('failed loading ' + url, true /* retry */)
      if (res && res.status >= 400 && res.status < 500) return callback('failed loading ' + url, false /* no retry */)
      if (err) return callback(err, false)

But it could be tackled as a different bug I assume

[adrai]

This sounds all a bit hacky... So to recap... You have this only in cypress? I don't know cypress. I would love to have a small reproducable example. Is it possible for you to create it?

[defko]

Yep it is getting too confusing. I will put something together in next week.

Anyway thank you for the fast responses!

[adrai]

alternatively, you could try to omit the require of node-fetch, with something like https://webpack.js.org/plugins/ignore-plugin/

[defko]

Hi @adrai

I just pushed a really simple example to reproduce the issue: https://github.com/defko/i18next-xhr-webpack

[adrai]

should be fixed with v1.0.14

[defko]

Thank you @adrai

[adrai]

If you like this module don’t forget to star this repo. Make a tweet, share the word or have a look at our https://locize.com to support the devs of this project.

There are many ways to help this project 🙏

[defko]

Hi @adrai,

First of all thank you for the quick fix, but looks like there is still an issue with a current solution. You can check the same example. As you can see there, the translation is still not working.

After a quick debug the issue is the XHR request set the error even if the request is success. See loadUrl function in index.js,

 this.options.request(this.options, url, undefined, (err, res) => {
     // Here err==='OK'
      if (res && res.status >= 500 && res.status < 600) return callback('failed loading ' + url, true /* retry */)
      if (res && res.status >= 400 && res.status < 500) return callback('failed loading ' + url, false /* no retry */)
      if (err) return callback(err, false)

Do y want me to open a different bug for this?

[adrai]

sorry, I missed that... will fix it asap

[adrai]

should be fixed with v1.0.15

[defko]

Thanks! It is working flawlessly now

[adrai]

Nice to hear 👍