search

vercel / remix

Build Better Websites. Create modern, resilient user experiences with web fundamentals.
https://remix.run
MIT License
63 stars 20 forks source link

Enable installGlobals({ nativeFetch: true }) when singleFetch is active #122

Closed coji closed 2 months ago

coji commented 3 months ago

Issue Description

Since Remix 2.9, the singleFetch option has been introduced. When this option is enabled, it requires installGlobals({ nativeFetch: true }) to be set in the runtime for proper functionality. However, the current implementation of @vercel/remix doesn't allow for this configuration, causing issues when deploying to Vercel.

Current Behavior

  1. The singleFetch option works locally but fails when deployed to Vercel.
  2. When using Vercel AI SDK for streaming in a resource route action, the lack of installGlobals({nativeFetch:true}) causes the deployment to fail on Vercel (while working locally). ref: https://github.com/vercel/remix/issues/109
  3. @vercel/remix has hardcoded installGlobals(); in globals.ts on the server side, preventing application-level configuration: https://github.com/vercel/remix/blob/main/packages/vercel-remix/globals.ts#L2

Workaround

Currently, we're applying a patch to server.js using pnpm patch to enable the required functionality: https://github.com/techtalkjp/techtalk.jp/pull/27/files

However, this workaround requires updating the patch every time the package version changes, which is cumbersome and not sustainable.

Proposed Solution

We request a modification to @vercel/remix that automatically enables installGlobals({ nativeFetch: true }) when singleFetch is active during server execution. Specifically:

  1. Detect when singleFetch is enabled in the Remix configuration.
  2. If singleFetch is active, use installGlobals({ nativeFetch: true }) instead of the current hardcoded installGlobals();.

This change would resolve the current issues and prepare for future compatibility, especially considering the direction of Remix v3.

Future Considerations

Given that Remix v3 (with React Router v7) is expected to have singleFetch enabled by default, the current implementation of @vercel/remix will likely need to change regardless. This issue presents an opportunity to address both current and future needs.

Questions

  1. Is there a plan to implement this automatic nativeFetch: true setting when singleFetch is active in @vercel/remix?
  2. If so, what is the timeline for this implementation?
  3. In the meantime, is there a recommended approach to enable these features without resorting to patching?

Thank you for your attention to this matter. We look forward to your response and any potential solutions.

coji commented 3 months ago

I apologize for the confusion in my original post. After further investigation, I've discovered some new information that I'd like to share as an update to this issue:

  1. The patch I mentioned earlier was not actually applied successfully. This explains why the initial solution didn't work as expected.

  2. Upon further investigation of the Vercel Remix builder source code, I discovered an undocumented environment variable VERCEL_REMIX_NATIVE_FETCH that appears to control the nativeFetch behavior. https://github.com/vercel/vercel/blob/main/packages/remix/defaults/server-node.mjs#L10

  3. I tried setting VERCEL_REMIX_NATIVE_FETCH=1 in the Vercel environment variables. This seemed to enable the use of undici for fetch operations, which was the intended behavior.

  4. However, this change led to a new error in Vercel Serverless Functions:

    TypeError: RequestInit: duplex option is required when sending a body.

    The full error stack trace is as follows:

    TypeError: RequestInit: duplex option is required when sending a body.
 at new Request (/var/task/node_modules/.pnpm/undici@6.19.2/node_module
s/undici/lib/web/fetch/request.js:539:15)
 at createRemixRequest (file:///var/task/build/server/nodejs-eyJydW50aW1
lIjoibm9kZWpzMTRfeCIsInJlZ2lvbiI6InN6LWRhMDEiLCJpc0RldiI6ZmFsc2V9/server-index.mjs:61:10)
 at Server.default (file:///var/task/build/server/nodejs-eyJydW50aW1lIjo
ibm9kZWpzMTRfeCIsInJlZ2lvbiI6InN6LWRhMDEiLCJpc0RldiI6ZmFsc2V9/server-index.mjs:80:19)
 at /opt/rust/nodejs.js:7:3792
 at AsyncLocalStorage.run (node:async_hooks:346:14)
 at /opt/rust/nodejs.js:7:3780
 at AsyncLocalStorage.run (node:async_hooks:346:14)
 at Server.<anonymous> (/opt/rust/nodejs.js:7:3768)
 at Server.s (/opt/rust/nodejs.js:7:825)
 at Server.emit (node:events:519:28)

Given these new findings, I have some additional questions:

  1. Can you confirm the functionality and intended use of the VERCEL_REMIX_NATIVE_FETCH environment variable?
  2. Are there plans to document this variable and its usage in the official @vercel/remix documentation?
  3. Is this new error (related to the duplex option) a known issue when using VERCEL_REMIX_NATIVE_FETCH=1 in Vercel Serverless Functions?
  4. Are there any additional configuration steps required when enabling VERCEL_REMIX_NATIVE_FETCH to ensure compatibility with Vercel Serverless Functions?

I apologize again for any confusion caused by my initial post. I hope this additional information helps in addressing the underlying issues. Thank you for your continued support and attention to this matter.

TooTallNate commented 2 months ago
  1. Can you confirm the functionality and intended use of the VERCEL_REMIX_NATIVE_FETCH environment variable?

VERCEL_REMIX_NATIVE_FETCH env var is indeed for the purpose of setting nativeFetch: true. Feel free to use it today.

  1. Are there plans to document this variable and its usage in the official @vercel/remix documentation?

To be determined. We're planning on evaluating whether it's safe to enable by default under certain conditions, but haven't gotten that far yet. It may end up remaining opt-in via this env var though, in which case we would document it.

  1. Is this new error (related to the duplex option) a known issue when using VERCEL_REMIX_NATIVE_FETCH=1 in Vercel Serverless Functions?

I haven't seen that one personally. If you could provide a repro then we could take a look.

  1. Are there any additional configuration steps required when enabling VERCEL_REMIX_NATIVE_FETCH to ensure compatibility with Vercel Serverless Functions?

Nope. Set the env var and then native fetch will be used in your serverless functions.

coji commented 2 months ago

@TooTallNate

Thank you for the detailed explanation. This information is very helpful.

VERCEL_REMIX_NATIVE_FETCH env var is indeed for the purpose of setting nativeFetch: true. Feel free to use it today.

Nope. Set the env var and then native fetch will be used in your serverless functions.

I understand points 1, 2, and 4 clearly now. The clarification on the use and implementation of VERCEL_REMIX_NATIVE_FETCH is much appreciated.

Regarding point 3:

I haven't seen that one personally. If you could provide a repro then we could take a look.

It seems the issue occurs when attempting to fetch with streaming from OpenAI on the server side. I'll create a reproduction and open a separate issue to address this specific problem in more detail.

Thank you again for your support and attention to this matter.