Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

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.

None yet

2 participants