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

fix: jokes tutorial actions should return 400 status for errors #895

Conversation

Graham42
Copy link
Contributor

@Graham42 Graham42 commented Dec 5, 2021

Why make this change?

When a request is sent to the server and there's something invalid about
the request, the server should return a 4xx status to accurately
represent the response.

We could have all the error cases use the json function directly, but
then we loose the advantage of the helpful typechecking ActionData
provides.

export const action: ActionFunction = async ({ request }) => {
  //...
  return json({ wrongActionDataErrorKey: "oops!" }, { status: 400 }));

For this reason, I've introduced the badRequest helper function so
that we can still get typechecking for ActionFunction returns, while
returning the accurate status in the Response.

const badRequest = (data: ActionData) => json(data, { status: 400 });

export const action: ActionFunction = async ({ request }) => {
  //...
  // and then TypeScript will alert us
  return badRequest({ wrongActionDataErrorKey: "oops!" });
  //                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Reference: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/400

Notes for the reviewer

I'm pretty sure I've updated all the line highlights correctly, but if you have any way of rendering a preview of the docs, that would help to increase confidence.

Thanks for your time reviewing this! 🙏

When a request is sent to the server and there's something invalid about
the request, the server should return a 4xx status to accurately
represent the response.

We could have all the error cases use the `json` function directly, but
then we loose the advantage of the helpful typechecking `ActionData`
provides.

```ts
export const action: ActionFunction = async ({ request }) => {
  //...
  return json({ wrongActionDataErrorKey: "oops!" }, { status: 400 }));
```

For this reason, I've introduced the `badRequest` helper function so
that we can still get typechecking for `ActionFunction` returns, while
returning the accurate status in the Response.

```ts
const badRequest = (data: ActionData) => json(data, { status: 400 });

export const action: ActionFunction = async ({ request }) => {
  //...
  // and then TypeScript will alert us
  return badRequest({ wrongActionDataErrorKey: "oops!" });
  //                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
```

Reference: https://developer.mozilla.org/en-US/docs/Web/HTTP/Status/400
@Graham42 Graham42 force-pushed the jokes-tutorial-fix-response-status-form-validation branch from ebe7a7d to 152d94c Compare December 5, 2021 03:02
@remix-cla-bot
Copy link
Contributor

remix-cla-bot bot commented Dec 5, 2021

Thank you for signing the Contributor License Agreement. Let's get this merged! 🥳

Copy link
Member

@kentcdodds kentcdodds left a comment

Choose a reason for hiding this comment

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

Super 👍 Thank you for this improvement 👏

@kentcdodds kentcdodds merged commit 44d4cb4 into remix-run:main Dec 6, 2021
@Graham42 Graham42 deleted the jokes-tutorial-fix-response-status-form-validation branch December 6, 2021 16:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants