TypeError: Cannot read properties of undefined (reading 'Symbol(Body internals)')

[gorankarlic]

Reproduction

This works

const response = await fetch(`https://google.com/`);
await response.text();

while the syntatically equivalent

const {text} = await fetch(`https://google.com/`);
await text();

fails with TypeError: Cannot read properties of undefined (reading 'Symbol(Body internals)').

Expected behavior

Both code blocks should work as they are syntactically equivalent. It might be related to NodeJS primordials, see NodeJS Issue #36364.

Your Environment

software	version
node-fetch	3.2.10
node	v18.9.0
npm	8.19.1
Operating System	MacOS x64

[leehambley]

We are affected by this too in a simple ts-fp project using node-fetch (which seems to be a noop, we just get the types from it). Something about whatever ts-fp is doing inside clearly resembles the destructuring style.

Using node-fetch because without it typescript with ts-fp complains about lack of types.

[jimmywarting]

We should not do anything to fix this. cuz this can't / shouldn't work even if we could solve this issue and allow for text and other methods to be deconstructed.

Both code blocks should work as they are syntactically equivalent

No they aren't. You will lose the context of what the function belongs to. an example of this could be

class C {
  f() {
    console.log('ctx is:', this)
  }
}
new C().f() // > ctx is: C { }
var {f} = new C(); f() // > ctx is: undefined

We arn't doing any brand checking/validation (as we should be doing anyway - but haven't gone around at fixing it)

The actual "Expected behavior" would would rather be an error with:
TypeError: Failed to execute 'text' on 'Response': Illegal invocation

There are many other things this aplies to. It's basically on all kind of methods off any es6 class.
An built in browser function of this is for example var {createElement} = document; createElement('foo') <-- produces the same TypeError

heck even test this out in browser too see if it also works: (hint it dose not work)

var {text} = new Response('x')
text() // gives a TypeError Illegal invocation (as expected)

if you want this kind of destructoring to work, then you would have to use bind or call

var response = new Response('x')
var { text } = response
text = text.bind(response); text() // works
await text.call(response) // or do this...

[jimmywarting]

closing this as the "won't fix"

if anything there should be an issue of "Do type brand checking and throw TypeError if it's decoupled of its own class that it belongs to" kind of issue

Bu doing something like

text() {
  if (!(this instanceof Response)) {
    throw new TypeError(...)
  }
}

[jimmywarting]

Sometimes i think developer over abuse destructoring too much for literal everything and making stuff less readable...

Even the minified destructoring version will be larger sometimes.

window.onmessage = ({data}) => console.log(data) // not worth it...
window.onmessage = e => console.log(e.data) // this is shorter

You will also get lost in the "of what context is this xyz doing?" kind of senarios. (you loose a namespace)
And also be in conflict of other similar names

// Somewhere at the top of the file:
const {encode} = new TextEncoder()
const {encode} = require('to-hex')
const {join} = require('node:path')

// later way down at the bottom of your script you do:
join(...xyz) // ...and you ask yourself where dose join come from? is it a array join method?

It makes it more readable / understandable if you had a tiny prefix namespace:

const textDecoder = new TextDecoder()
const toHex = require('to-hex')
const fs = require('node:path')

fs.join('a', 'b')
textDecoder.decode(x)
toHex(xyz)

Destructoring should mostly only be done

• when there is a need to "get rid of the rest" as to allow the engine to garbage collect the other things it dosen't need anymore or for three shaking
• and where you are calling/accessing a function or variable multiple times.

// There is no need to destruct fs or path as the hole `fs` & path will be GC'ed afterwards
// Unless you want to ship a browserified web bundle and do tree shaking

import fs from 'node:fs'
import path from 'node:path'

const file = path.join(import.meta.url, '../readme.md')
const pkg = fs.readFileSync(file)

export default pkg

// make since to destructor fs cuz you only need one thing and you are reusing it
import { readFileSync } from 'node:fs'

export function readJson(path) {
  return readFileSync(path)
}

[jimmywarting]

Worth reading https://www.aleksandrhovhannisyan.com/blog/overzealous-destructuring/

[leehambley]

Thanks for the comprehensive response @jimmywarting - I learned a lot. Of course losing the binding context makes sense here, and the error as presented did't immediately make sense, but after reading your notes, it's perfectly understandable. Thanks also for the reading suggestions and thorough explanation through code snippets.