-
Notifications
You must be signed in to change notification settings - Fork 14
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
Use react router for landing => checkout => thank-you route #5962
Conversation
const router = createBrowserRouter( | ||
geoIds | ||
.map((geoId) => [ | ||
{ |
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.
We'll add the /${geoId}
soon as part of the migration from /{geoId}/contribute
work as this URL is currently redirected via Fastly and the Play app.
Size Change: +14.9 kB (+1%) Total Size: 1.95 MB
ℹ️ View Unchanged
|
ssrPages: 'helpers/rendering/ssrPages.ts', | ||
}, | ||
common: { | ||
'[countryGroupId]/router': 'pages/[countryGroupId]/router.tsx', |
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.
This is the change - the rest is formatting.
type Props = { | ||
geoId: GeoId; | ||
}; | ||
function CheckoutComponent({ geoId }: Props) { |
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.
The ordering of this file has changed - that's provably the most important piece to review.
@GHaberis - re: filesize - currently It's not a fair comparison though as Interestingly - adding As a point, another reason I am not going to deep on exploring other routers is we have quite a few methods that use react-router with pageviews and the like. |
fd637fd
to
498acd7
Compare
498acd7
to
da2a059
Compare
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.
👍
Nice work!
Seen on PROD (merged by @jamesgorrie 9 minutes and 49 seconds ago)
Sentry Release: support-client-side, support |
react-router
to encapsulate thelanding => checkout => thank-you
journeyGeoIdConfig
which is config that is variable on the/{geoId}
segment of the URLcheckout
name for the route torouter
in the play appgeoId
in the checkout component into the componentI'm doing this because when setting up a whole new route for the thank you page from Play => rendering, the boilerplate was becoming immense and unwieldy. This replicates the pattern we had setup for
/{geoId}/contribution
from Play => Router.This also avoids having to spin up another
webpack.entryPoint
. A 🚤 performance-point 🚀 herePart of #5722
Tasks