Skip to content

docs(README): when patching global context, patch TextDecoder and TextEncoder too#1774

Open
jtomaszewski wants to merge 1 commit intonode-fetch:mainfrom
jtomaszewski:patch-1
Open

docs(README): when patching global context, patch TextDecoder and TextEncoder too#1774
jtomaszewski wants to merge 1 commit intonode-fetch:mainfrom
jtomaszewski:patch-1

Conversation

@jtomaszewski
Copy link

After following the README on patching the global context, node-fetch fails to me in jest jsdom environment on node v16.16.0 with following error:

TextDecoder is not defined

      Cause: ReferenceError: TextDecoder is not defined
      at Response.TextDecoder (node_modules/node-fetch/src/body.js:159:14)
      at call (node_modules/@babel/runtime/helpers/regeneratorRuntime.js:44:17)
      at Generator.tryCatch (node_modules/@babel/runtime/helpers/regeneratorRuntime.js:125:22)
      at Generator._invoke [as next] (node_modules/@babel/runtime/helpers/regeneratorRuntime.js:69:21)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:3:24)
      at asyncGeneratorStep (node_modules/@babel/runtime/helpers/asyncToGenerator.js:22:9)

That's probably because node-fetch in https://github.com/node-fetch/node-fetch/blob/main/src/body.js#L159 refers to TextDecoder using a global reference, which doesn't exist in my environment.

Changes

In recommended script in README that patches the global context, add TextDecoder and TextEncoder to the global context too.

I also removed a few unnecessary imports from it that weren't used at all.

Additional information

  • I updated readme

Current script contains unnecessary imports.

It also doesn't patch `TextDecoder` and `TextEncoder`, which may be referred to using global references while the fetch is happening.
@jtomaszewski jtomaszewski changed the title docs(README): improve script for patching the global object docs(README): when patching global context, patch TextDecoder and TextEncoder too Sep 7, 2023
@jimmywarting
Copy link
Collaborator

Since this is just readme and no breaking changes, then i would now have suggested another approach.

using conditional lazy top level import() and maybe Nullish coalescing assignment (??=) in order to not load it in vain

So maybe something like this:

// fetch-polyfill.js
globalThis.fetch || await import('node-fetch').then(mod => {
  globalThis.fetch = mod.default
  globalThis.Blob ??= mod.Blob
  globalThis.File ??= mod.File
  globalThis.FormData ??= mod.FormData
  globalThis.Headers = mod.Headers
  globalThis.Request = mod.Request
  globalThis.Response = mod.Response
})

globalThis.TextDecoder || import('node:util').then(util => {
  globalThis.TextDecoder ??= util.TextDecoder
  globalThis.TextEncoder ??= util.TextEncoder
})

// index.js
(globalThis.TextDecoder && globalThis.fetch) || await import('./fetch-polyfill.js')

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants