-
-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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: convert types in @remix-run/router to be framework-agnostic #9141
Conversation
|
@@ -27,7 +32,6 @@ import type { | |||
HashHistory, | |||
History, | |||
HydrationState, | |||
RouteObject, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
react-router-dom
now re-exports from react-router
instead of @remix-run-/router
@@ -631,6 +635,7 @@ export function createRoutesFromChildren( | |||
loader: element.props.loader, | |||
action: element.props.action, | |||
errorElement: element.props.errorElement, | |||
hasErrorBoundary: element.props.errorElement != null, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
createRoutesFromChildren
fills this field in for you automatically based on errorElement
. This means the user-facing JSX API doesn't change at all.
If a user decides to create their own routes
array manually and do <DataBrowserRouter routes={routes} />
, then they are responsible for including hasErrorBoundary
accordingly. This feels sort of annoying but seems necessary unless we want to walk the tree and add it on our own prior to handing off to createRouter
- which means we need to create copies of the routes to avoiding mutating in place, etc.
@ryanflorence WDYT?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I'd like to add it before handing it off to createRouter
. That shouldn't be a detail React Router users should have to care about.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ok cool - added that in 11b9bb6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and renamed to enhanceManualRouteObjects
in b45f4f5 😄
RouteObjectType extends RouteObject = RouteObject | ||
> extends AgnosticRouteMatch<ParamKey, RouteObjectType> {} | ||
|
||
export interface DataRouteMatch extends RouteMatch<string, DataRouteObject> {} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Export react-specific versions of these from react-router
@@ -559,7 +560,7 @@ export function _renderMatches( | |||
if (dataRouterState?.errors) { | |||
// Don't bail if we have data router errors so we can render them in the | |||
// boundary. Use the pre-matched (or shimmed) matches | |||
matches = dataRouterState.matches; | |||
matches = dataRouterState.matches as DataRouteMatch[]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Since we're inside a data router initialized with react-specific routes, we can safely cast these back here when we pull them from the agnostic router state.
c8d1289
to
e519220
Compare
Removes the last two
React.ReactNode
type references from@remix-run/router
. It doesn't need to know or care about theelement
/errorElement
in the end, it simply needs to know what routes have error boundaries so it can bubble errors accordingly.Changes:
element
/errorElement
removed from@remix-run/router
types in favor ofhasErrorBoundary?: boolean
@remix-run/router
renamed toAgnosticRouteObject
,AgnosticDataRouteObject
,AgnosticRouteMatch
, andAgnosticDataRouteMatch
for clarityreact-router
extends these types with react-specificelement
/errorElement
fields and exports the current public-API types ofRouteObject
,DataRouteObject
,RouteMatch
, andDataRouteMatch
I'm not crazy about the
Agnostic*
prefixes but it makes it very explicit that these are not the things you should be using in your application code, and you should always be using the non-agnostic types exported fromreact-router
(or eventually@remix-run/preact
,@remix-run/vue
, etc.).