fix: update thrown/returned Response handling in staticHandler #9465
Conversation
🦋 Changeset detectedLatest commit: 3516c17 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
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 |
| @@ -10237,6 +10237,20 @@ describe("a router", () => { | |||
| }; | |||
| } | |||
|
|
|||
| it("should match routes automatically if no routeId is provided", async () => { | |||
There was a problem hiding this comment.
@jacob-ebey This oughta allow us to simplify the resource route usage in RRR
| } catch (data) { | ||
| expect(data.status).toBe(201); | ||
| expect(await data.text()).toBe("Created!"); | ||
| } |
There was a problem hiding this comment.
any non-redirect thrown response will throw from queryRoute
| expect(data instanceof Response).toBe(true); | ||
| expect(data.status).toBe(404); | ||
| expect(data.statusText).toBe("Not Found"); | ||
| expect(data.headers.get("X-Remix-Router-Error")).toBe("yes"); |
There was a problem hiding this comment.
Router-generated errors (404 and 405) will now contain a custom header we can leverage in remix as needed
There was a problem hiding this comment.
This should cover our use-case :)
| * redirect response is returned or thrown from any action/loader. We | ||
| * propagate that out and return the raw Response so the HTTP server can | ||
| * return it directly. | ||
| */ |
There was a problem hiding this comment.
@jacob-ebey Let me know if this and the comment above queryRoute clarify the behavior (and match what we discussed yesterday).
The one thing that's slightly different I think is that while in the remix use-case, callRouteAction for example will never throw a user-thrown error - this remix-agnostic code will still intend to throw user-thrown things inside of queryRoute. We just shouldn't hit that path in the Remix usage of remix-run/router
| let location = createLocation("", createPath(url), null, "default"); | ||
| let matches = matchRoutes(dataRoutes, location); | ||
|
|
||
| if (!matches) { |
There was a problem hiding this comment.
Lift 404 handling up to query/queryRoute to avoid branching inside queryImpl
| status: error.status, | ||
| statusText: error.statusText, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Not a possible path anymore now that we throw 404/405 Response's directly
| status: value.status, | ||
| statusText: value.statusText, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Not a possible path either since all isRouteRequest's throw inside callLoaderOrAction
Updates to
createStaticHandlerto better normalize the return values ofquery/queryRoutefor use in Remix