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

feat: export proper ErrorResponse type #10811

Merged
merged 1 commit into from
Aug 23, 2023

Conversation

brophdawg11
Copy link
Contributor

@brophdawg11 brophdawg11 commented Aug 23, 2023

This handles the callout in remix-run/remix#7238 which has come up a few times in discord as well.

We always wanted class ErrorResponse to be an implementation detail since it serves no runtime purpose in userland. We naively thought isRouteErrorResponse's type narrowing would be sufficient, and is in the simple case:

function ErrorBoundary() {
  let error = useRouteError();
  // 🙁 error is un-typed

  if (isRouteErrorResponse(error)) {
    // yay typed! 😀
  }
}

However, this is limited since you can't build abstractions around these errors:

function ErrorBoundary() {
  let error = useRouteError();

  if (isRouteErrorResponse(error)) {
    return <MySuperCoolErrorResponseUI error={error}
  }
}

// ❌ Uh-oh - we don't have the class available to us to type this function
function MySuperCoolErrorResponseUI(error: any) {
  return <>{/*...*/}</>;
}

The class was always marked @private and wasn't ever intended to be used, so this PR moves the class to an UNSAFE_ErrorResponseImpl export. Because it was always marked "private", I don't consider this a breaking change.

That frees up the ErrorResponse name so we can export a type of an instance of the class, which properly excludes the private properties:

export type ErrorResponse = InstanceType<typeof ErrorResponseImpl>;

Then we can export this ErrorResponse type out through @remix-run/react as well.

Main changes are in utils.ts - the rest is just bubbling the name changes out and exporting the new type out through the dependent packages

@changeset-bot
Copy link

changeset-bot bot commented Aug 23, 2023

🦋 Changeset detected

Latest commit: 356f2a5

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

This PR includes changesets to release 5 packages
Name Type
react-router-dom-v5-compat Patch
react-router-native Patch
react-router-dom Patch
react-router Patch
@remix-run/router 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

Comment on lines +1512 to +1513
private error?: Error;
private internal: boolean;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

These should have been private all along, but since the entire class was a "private" implementation detail I think I glossed over the field specifiers. Marking them private ensures the type below doesn't include them.

@MichaelDeBoey MichaelDeBoey changed the title Export proper ErrorResponse type feat: export proper ErrorResponse type Aug 23, 2023
@brophdawg11 brophdawg11 merged commit a4495de into dev Aug 23, 2023
3 checks passed
@brophdawg11 brophdawg11 deleted the brophdawg11/error-response-type branch August 23, 2023 20:39
@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 6.16.0-pre.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

@github-actions
Copy link
Contributor

🤖 Hello there,

We just published version 6.16.0 which includes this pull request. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!

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