Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resource Route Single Fetch Updates #9349

Merged
merged 6 commits into from May 1, 2024

Conversation

brophdawg11
Copy link
Contributor

@brophdawg11 brophdawg11 commented Apr 30, 2024

When single fetch is enabled:

  • Raw javascript objects returned from resource routes are automatically wrapped in json() and a deprecation warning is logged
  • Resource routes will receive the response stub for consistency and it will be respected, but the preferred way to handle status/headers is via your returned Response

Closes #9314

Copy link

changeset-bot bot commented Apr 30, 2024

🦋 Changeset detected

Latest commit: 19a4b17

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 16 packages
Name Type
@remix-run/server-runtime Patch
@remix-run/cloudflare Patch
@remix-run/deno Patch
@remix-run/dev Patch
@remix-run/node Patch
@remix-run/react Patch
@remix-run/testing Patch
@remix-run/cloudflare-pages Patch
@remix-run/cloudflare-workers Patch
@remix-run/architect Patch
@remix-run/express Patch
@remix-run/serve Patch
create-remix Patch
remix Patch
@remix-run/css-bundle Patch
@remix-run/eslint-config Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@brophdawg11 brophdawg11 force-pushed the brophdawg11/single-fetch-resource-routes branch from 1c641bf to cc1aed7 Compare April 30, 2024 19:43
@brophdawg11 brophdawg11 changed the title Wrap resource route raw object return in json for v2 back compat Resource Route Single Fetch Updates Apr 30, 2024
@brophdawg11 brophdawg11 force-pushed the brophdawg11/single-fetch-resource-routes branch from cc1aed7 to beef932 Compare April 30, 2024 20:12
@brophdawg11 brophdawg11 force-pushed the brophdawg11/single-fetch-resource-routes branch from 9f2dff9 to a299c07 Compare April 30, 2024 20:31
@brophdawg11 brophdawg11 mentioned this pull request Apr 30, 2024
@manzano78
Copy link
Contributor

@brophdawg11, does it mean that we can’t benefit from turbo-stream Date/Set/Map/Promise types in a useFetcher which load()s a resource route?

…r of LoaderFunctionArgs_SingleFetch"

This reverts commit a299c07.
@brophdawg11
Copy link
Contributor Author

@manzano78 No - when a fetcher hits a resource route it will still make a /resource.data request and it'll go through the turbo-stream-enabled code path. If you were to CURL a resource route, you'd get this json() behavior

@manzano78
Copy link
Contributor

@brophdawg11 yes I can reproduce the turbo-stream format but since json() applies a simple JSON.stringify(data), I still receive a string instead of a Date in my fetcher's data :-/

@brophdawg11
Copy link
Contributor Author

I'm not sure what the issue is. Fetchers can load from resource routes returning raw objects and they will be streamed using turbo-stream, which means you will not lose Date instances: https://stackblitz.com/edit/remix-run-remix-beoo1t?file=app%2Froutes%2F_index.tsx

This PR does not change that behavior when accessed via a fetcher.

If you are still having issues, would you be able to please open a github issue with a minimal reproduction?

@brophdawg11 brophdawg11 merged commit 09e81a8 into dev May 1, 2024
9 checks passed
@brophdawg11 brophdawg11 deleted the brophdawg11/single-fetch-resource-routes branch May 1, 2024 18:55
@github-actions github-actions bot added the awaiting release This issue has been fixed and will be released soon label May 1, 2024
}
```

To keep things consistent, resource route `loader`/`action` functions will still receive a `response` stub and you can use it if you need to (maybe to share code amongst non-resource-route handlers):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great improvement, thank you! I ran into this after I refactored our authenticate helper to take the response stub to append the Set-Cookie header, so it's really nice to be able to use that from resource routes as well.

"@remix-run/server-runtime": patch
---

Pass `response` stub to resource route handlerså when single fetch is enabled
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is an errant å in the changset text.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
awaiting release This issue has been fixed and will be released soon CLA Signed package:server-runtime
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Single fetch response parameter for loader is undefined if no default export exists in the route
4 participants