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.

Sign up for GitHub

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

Jump to bottom

Bumps react-router-dom to v6 #2256

Merged
merged 37 commits into from
Sep 30, 2024
Merged

Bumps react-router-dom to v6 #2256

merged 37 commits into from
Sep 30, 2024

Conversation

infomiho
Copy link
Contributor

@infomiho infomiho commented Aug 27, 2024 •
edited
Loading

Closes #2245

Left to do

  • Update the type-safe Link component
    • Optional static segment
    • Splat routes
    • .build method on the route
  • Test all example apps
  • Test all starters
  • Update docs
    • Update API usage (e.g. useHistory is replaced with useNavigate)
    • Update <Link /> component docs
      • Talk about the "splat" route
      • Talk about optional static segments syntax
    • Update tutorial not to use match.params
    • Update rootComponent to use <Outlet /> instead of children
  • Explore using lazy loading by default for pages - f65f929
  • Explore using a non-default 404 error page (default image below)
Screenshot 2024-09-09 at 15 26 37
Default 404 page in dev
Screenshot 2024-09-09 at 15 28 51
Default 404 page in prod

Breaking changes

  • useHistory() -> useNavigate()
  • <Redirect /> -> <Navigate />
  • Wasp's root component feature requires rendering <Outlet /> instead of rendering children
  • props.match.params -> useParams()

@infomiho
Bump React Router DOM to v6
0280489 
Signed-off-by: Mihovil Ilakovac <[email protected]>
@infomiho
e2e tests
0e712fc 
Signed-off-by: Mihovil Ilakovac <[email protected]>
infomiho
infomiho commented Aug 27, 2024
View reviewed changes
waspc/data/Generator/templates/react-app/src/auth/pages/OAuthCallback.tsx
@@ -14,7 +14,7 @@ export function OAuthCallbackPage() {
const { error, user } = useOAuthCallbackHandler();

if (user !== undefined && user !== null) {
return <Redirect to="{= onAuthSucceededRedirectTo =}" />;
return <Navigate to="{= onAuthSucceededRedirectTo =}" replace />;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

<Navigate /> replace <Redirect /> and the default behaviour changed, we need to add replace explicitly.

Copy link
Contributor

Choose a reason for hiding this comment

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

If you got links to the changelog that prescribes this, you can include it into these comments.

I'm not sure is it worth spending the time, but it would help me better judge the change's validity.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://reactrouter.com/en/main/upgrading/v5#use-usenavigate-instead-of-usehistory

If you prefer to use a declarative API for navigation (ala v5's Redirect component), v6 provides a Navigate component.

infomiho
infomiho commented Sep 3, 2024
View reviewed changes
waspc/data/Generator/templates/react-app/src/router.tsx Outdated
}])


export const router = <RouterProvider router={browserRouter} />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

From the docs:

You should only opt into the Component API for data routes via RouterProvider. Using this API on a inside will de-optimize React's ability to reuse the created element across renders.

Since we are using Component and not element API, using the RouterProvider is optimal.

@infomiho infomiho marked this pull request as ready for review September 10, 2024 14:22
infomiho
infomiho commented Sep 12, 2024
View reviewed changes
examples/websockets-realtime-voting/src/Layout.tsx
@@ -61,7 +62,9 @@ export const Layout = ({ children }: { children: ReactNode }) => {
<Navbar.Link href="#">Contact</Navbar.Link>
</Navbar.Collapse> */}
</Navbar>
<div className="grid place-items-center mt-8">{children}</div>
<div className="grid place-items-center mt-8">
<Outlet />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Breaking change: children -> <Outlet />

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, children are often used as an emotional outlet for their parents, checks out.

@infomiho infomiho mentioned this pull request Sep 12, 2024
Open
infomiho
infomiho commented Sep 12, 2024
View reviewed changes
waspc/data/Generator/templates/react-app/src/components/DefaultRootErrorBoundary.tsx

import { FullPageWrapper } from './FullPageWrapper'

export function DefaultRootErrorBoundary() {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We are using a default root error boundary to make it look nicer when rendering fails. In the future, we should allow customisation of the root error boundary: #2279

infomiho
infomiho commented Sep 12, 2024
View reviewed changes
waspc/data/Generator/templates/sdk/wasp/client/router/types.ts
@@ -21,7 +21,7 @@ export type Search =

type RouteDefinitionsToRoutesObj<Routes extends RoutesDefinition> = {
[K in keyof Routes]: {
to: Routes[K]['to']
to: ExpandRouteOnOptionalStaticSegments<Routes[K]['to']>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a bit of Typescript type level programming magic. It converts string literals that have a route with optional static segments into a union of string literals of possible route variants.

ExpandRouteOnOptionalStaticSegments<'/my/:id/route?'> will result in '/my/:id' | '/my/:id/route' type.

sodic
sodic reviewed Sep 24, 2024
View reviewed changes
Copy link
Contributor

@sodic sodic left a comment

Choose a reason for hiding this comment

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

Nice work!

I'm still looking at the types file, but more out of curiosity than necessity.

If we fix everything else, it can go as is.

examples/thoughts/src/client/MainPage.jsx Outdated Show resolved Hide resolved
examples/websockets-realtime-voting/src/Layout.tsx Show resolved Hide resolved
waspc/data/Generator/templates/sdk/wasp/client/router/index.ts Outdated Show resolved Hide resolved
waspc/data/Generator/templates/sdk/wasp/client/router/linkHelpers.ts Show resolved Hide resolved
web/docs/advanced/links.md Show resolved Hide resolved
waspc/src/Wasp/Util/WebRouterPath.hs Outdated Show resolved Hide resolved
waspc/src/Wasp/Generator/SdkGenerator.hs Outdated Show resolved Hide resolved
waspc/src/Wasp/Generator/SdkGenerator.hs Show resolved Hide resolved
waspc/data/Generator/templates/react-app/src/router.tsx Show resolved Hide resolved
waspc/data/Generator/templates/sdk/wasp/client/router/types.ts Outdated Show resolved Hide resolved
sodic
sodic reviewed Sep 27, 2024
View reviewed changes
waspc/src/Wasp/Util/WebRouterPath.hs Outdated
else RequiredStaticSegment segmentValue

isSegmentOptional :: String -> Bool
isSegmentOptional = isSuffixOf "?"
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 definitely a candidate for a local where definition.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call, I agree 👍

@infomiho infomiho requested a review from sodic September 27, 2024 18:10
@infomiho infomiho merged commit d1f0c79 into main Sep 30, 2024
6 checks passed
@infomiho infomiho deleted the miho-react-router-dom-6 branch September 30, 2024 11:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sodic sodic sodic approved these changes

@Martinsos Martinsos Awaiting requested review from Martinsos

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update React router to the latest version
2 participants
@infomiho @sodic