Single Fetch Issues

[michaelgmcd]

Note: I'm still working on the migration/testing, so I kept this title vague for now in case I come across other problems.

Reproduction

https://stackblitz.com/edit/remix-run-remix-tfwhw2?file=app%2Froutes%2F_index.tsx

System Info

Vite: 5.2.10
Remix: 2.9.1

System:
    OS: macOS 14.4.1
    CPU: (10) arm64 Apple M2 Pro
    Memory: 80.28 MB / 32.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.12.2 - /opt/n/bin/node
    npm: 10.5.0 - /opt/n/bin/npm
    pnpm: 8.15.7 - /opt/homebrew/bin/pnpm
    Watchman: 2024.04.08.00 - /opt/homebrew/bin/watchman
  Browsers:
    Chrome: 124.0.6367.78
    Safari: 17.4.1

Used Package Manager

pnpm

Expected Behavior

Happy to split this out into separate issues, but while migrating to Single Fetch, I've found two issues:

1. From the Single Fetch overview doc:

If you are currently returning Response instances from your loaders (i.e., json/defer) then you shouldn't need to make many changes to your app code, but please read through the "breaking" changes below to be aware of some of the underlying behavior changes - specifically around serialization and status/header behavior.

This is in the reproduction, but for the existing use case with json(), the application crashes:

export const loader = ({ response }: LoaderFunctionArgs) => {
  return json({}, {
    headers: {
      'X-Test-Header': 'Foo-Bar'
    }
  })
}

json and other utilities now require the headers key to be an instance of Headers.

It could be as simple as checking if result.result.headers instanceof Headers before calling proxyResponseToResponseStub, or making the headers argument of proxyResponseToResponseStub something like headers: Headers | Record<string, string>, and handling it accordingly.

2. This is half question, half bug report.

From the Single fetch headers doc:

To alter the status of your HTTP Response, set the status field directly:
response.status = 201

However, the response stub is potentially undefined (the type in the doc calls this out as well). When is response stub undefined? Having to use a non-null assertion every time doesn't feel right, and typescript throws The left-hand side of an assignment expression may not be an optional property access. if trying to do something like response?.status = 200

Actual Behavior

1. The type of json would be correct and disallow plain objects for headers or support them
2. Clear documentation on when the response stub is undefined or improved ways of handling things like redirects (The doc mentions the current utilities are deprecated)

[michaelgmcd]

After looking at this a bit more, even a simple:

export const loader = () => {
  return json({})
}

throws the same error:

TypeError: headers.getSetCookie is not a function or its return value is not iterable
    at proxyResponseToResponseStub (file:///home/projects/remix-run-remix-tfwhw2/node_modules/@remix-run/server-runtime/dist/single-fetch.js:292:25)
    at eval (file:///home/projects/remix-run-remix-tfwhw2/node_modules/@remix-run/server-runtime/dist/single-fetch.js:61:9)
    at async Promise.all (index 1)
    at async eval (file:///home/projects/remix-run-remix-tfwhw2/node_modules/@remix-run/server-runtime/dist/single-fetch.js:39:19)
    at async callDataStrategyImpl (file:///home/projects/remix-run-remix-tfwhw2/node_modules/@remix-run/router/dist/router.cjs.js:4169:17)
    at async callDataStrategy (file:///home/projects/remix-run-remix-tfwhw2/node_modules/@remix-run/router/dist/router.cjs.js:3702:19)
    at async loadRouteData (file:///home/projects/remix-run-remix-tfwhw2/node_modules/@remix-run/router/dist/router.cjs.js:3677:19)
    at async queryImpl (file:///home/projects/remix-run-remix-tfwhw2/node_modules/@remix-run/router/dist/router.cjs.js:3522:20)
    at async Object.query (file:///home/projects/remix-run-remix-tfwhw2/node_modules/@remix-run/router/dist/router.cjs.js:3416:18)
    at async handleDocumentRequest (file:///home/projects/remix-run-remix-tfwhw2/node_modules/@remix-run/server-runtime/dist/server.js:222:15)

[michaelgmcd]

Found the culprit for the first issue. installGlobals without nativeFetch uses remix's own polyfill, which doesn't implement headers.getSetCookie. The docs aren't clear on whether or not installGlobals should be called for custom servers.

[coji]

I am having the same problem with vercel.

Here is my reproduction code.
https://github.com/coji/remix-vercel-single-fetch-test

I was wondering where to put installGlobals since it's not a custom server.

[Tsirimaholy]

I commented on the Vercel Fork, but I will leave it there in case it may help.
I'm getting this error too. I tried to use node 20 if it solves the issue, but it's the same.
The server is not accessible (Just blank screen).
Mine is just a local dev environment.
I tried to dig deeper a little bit and there I see this line:

remix/packages/remix-server-runtime/single-fetch.ts

Line 350 in 7b2fa03

for (let v of headers.getSetCookie()) {

Even the comments themselves acknowledge the presence of certain typing issues.
I want to contribute to solve this issue, but i'm not a yet great JS dev myself 😅 .

[brophdawg11]

Regarding the headers issue - It looks like we may have missed this call out in the docs and only included it in the release notes: https://github.com/remix-run/remix/blob/main/CHANGELOG.md#single-fetch-unstable

Single fetch requires using the built-in fetch on node 20, or passing the new option to installGlobals({ nativeFetch: true }) in your custom server.

I’ll get that added to the docs 👍

Update Added to the docs in #9335

[michaelgmcd]

Just came back here to say the same thing. Rereading the release notes and saw it.

[brophdawg11]

To recap here, I think this can be closed out?

1. was due to a missing installGlobals({ nativeFetch: true }) - we added a call out to the docs inhttps://github.com/Add section on undici polyfill to Single Fetch docs #9335.
2. is a duplicate of Single fetch response parameter for loader is undefined if no default export exists in the route #9314 and being addressed in Resource Route Single Fetch Updates #9349. We are adding the response stub to resource routes to eliminate the ambiguity, and I think we are also probably going to look into a LoaderFunctionArgs override in the future/single-fetch.d.ts file so we don't have to introduce response as optionl when apps have not opted into single fetch.

Please let me know if anything is still outstanding and we can re-open!

[sahithyandev]

I have enabled Single Fetch in my Remix (with Vite) project. It's working locally, but not on Vercel. I am seeing headers.getSetCookie is not defined error, and a blank page.

I am using Node 20. I am calling installGlobals({ nativeFetch: true }) in vite.config.ts

[michalmo]

There's an issue for the problem with Vercel at vercel#109 where I posted a workaround.