Moving this back to draft. It's going to require a compatibility flag which is going to require some additional work.

Moving this back to draft. It's going to require a compatibility flag which is going to require some additional work.

Do we really need a compat flag here?
I can not really think about a use case in which users were passing an (async) iterable and expect .toString() to be called on it.
I would rather consider this a bug fix because the new behavior is what Node does

Does the new behavior only apply to nodejs_compat - I don't immediately see that in the code

Yes, a compat flag is needed. The key issue is that stuff like new Response('hello') and new Response(['a','b','c']) are currently supported, it's also currently possible to pass something like new Response({ toString() { return 'abc' } })... specifically, strings and arrays are both iterables. This mechanism as written now handles strings as a special case but it becomes ambiguous when the user passes an object (like an Array) that implements both stringify and iterable semantics.

I can not really think about a use case in which users were passing an (async) iterable...

We have tests that do exactly this. Specifically, we have tests that are passing an Array to new Response(...). Arrays are both stringifiable (which is how they are handled now) and sync iterable. We cannot assume that users aren't doing similar things.

I would rather consider this a bug fix because the new behavior is what Node does

This really doesn't matter. Even as a bug fix it would require a compat flag.

Does the new behavior only apply to nodejs_compat - I don't immediately see that in the code

No, it's being enabled generally, not just for nodejs_compat.

Testing Node.js further, it looks like they cheat around the issue by actually not supporting sync iterables.

> function* gen() { yield 'a'; yield 'b'; }
undefined
> r = new Response(gen());
Response {
  status: 200,
  statusText: '',
  headers: Headers { 'content-type': 'text/plain;charset=UTF-8' },
  body: ReadableStream { locked: false, state: 'readable', supportsBYOB: true },
  bodyUsed: false,
  ok: true,
  redirected: false,
  type: 'default',
  url: ''
}
> r.text().then(console.log)
Promise {
  <pending>,
  Symbol(async_id_symbol): 226,
  Symbol(trigger_async_id_symbol): 218
}
> [object Generator]
> async function* gen() { yield 'a'; yield 'b'; }
undefined
> r = new Response(gen());
Response {
  status: 200,
  statusText: '',
  headers: Headers {},
  body: ReadableStream { locked: false, state: 'readable', supportsBYOB: true },
  bodyUsed: false,
  ok: true,
  redirected: false,
  type: 'default',
  url: ''
}
> r.text().then(console.log)
Promise {
  <pending>,
  Symbol(async_id_symbol): 360,
  Symbol(trigger_async_id_symbol): 349
}
> ab

So actually, I think we may be able to safely enable that case but there's still an ambiguity that probably still requires a compat flag... If an obj the user is currently passing is has a Symbol.asyncIterator property that is ignored currently, it would suddenly start being treated differently. Because we really don't know if those exist, we have to use a compat flag.

It's a node.js extension. As far as I know it's not supported by spec.

Given this is a Node.js compat feature, could we put it behind nodejs_compat?

Given this is a Node.js compat feature, could we put it behind nodejs_compat

We've had people request it independently of nodejs_compat. I was planning to imply it on when nodejs_compat is on but also wanted the ability to switch it on/off independently and have it available without nodejs_compat.

Right, and I know you mentioned this before but just to reiterate - the only risky compat case seems to be fetch('foo', { method: 'POST', body: [1,2] }). Is there any framing where we could special case that case outside of node_compat? Might be an option to keep in mind if this ever comes up.

Well, in that case they can simply disable the compat flag to switch it off. Not sure what other special casing would be necessary?

Yeah agreed, it seems like it should be fine, but there's options if we hit any compat questions down the line. I mean specifically separating treatment of body on fetch from the init on request/response if such a compat case came up.

Thanks @jasnell 🎉