sergiodxa / remix-utils

A set of utility functions and types to use with Remix.run
https://sergiodxa.github.io/remix-utils/
MIT License
2.14k stars 122 forks source link

eventStream triggers warnings in Remix with single fetch #400

Open kentcdodds opened 1 month ago

kentcdodds commented 1 month ago

Describe the bug

When Single Fetch is enabled, eventStream causes Remix to output an error:

⚠️ REMIX FUTURE CHANGE: Resource routes will no longer be able to return raw JavaScript objects in v3 when Single Fetch becomes the default. You can prepare for this change at your convenience by wrapping the data returned from your loader function in the routes/test route with json(). For instructions on making this change see https://remix.run/docs/en/v2.9.2/guides/single-fetch#resource-routes

Your Example Website or App

https://github.com/epicweb-dev/epicshop

Steps to Reproduce the Bug or Issue

  1. Create a Remix app with single fetch enabled
  2. Create a resource route that returns an event stream
  3. Notice the error

Expected behavior

I expect no warnings

Screenshots or Videos

image

Platform

Additional context

It's possible this is an error in Remix's detection of returned responses.

sergiodxa commented 1 month ago

eventStream returns a Response object, so maybe they consider it a raw object?

kentcdodds commented 1 month ago

Here's how they detect whether something is a response: https://github.com/remix-run/remix/blob/aabc7f84514c1c0e0ba8e33c48c7fba422cf8084/packages/remix-server-runtime/responses.ts#L107C1-L116C1

export function isResponse(value: any): value is Response {
  return (
    value != null &&
    typeof value.status === "number" &&
    typeof value.statusText === "string" &&
    typeof value.headers === "object" &&
    typeof value.body !== "undefined"
  );
}

Perhaps one of those things is unset in the eventStream?

sergiodxa commented 1 month ago

It has a headers object and body, but I never set a status and statusText, if that solves the issue it could be a quick change here, it can be just status 200 and statusText set to OK

kentcdodds commented 1 month ago

doesn't new Response default the status to 200 and statusText to OK?