Jetpack: update react-router-dom from v5 to v6

[tbradsha]

This is PR bumps Jetpack-the-plugin to use react-router-dom to v6 (and ultimately to v7).

Proposed changes:

Essentially I followed this guide:
https://reactrouter.com/6.28.0/upgrading/v5

Of note:

• <Prompt> is no longer supported in v6, so I've commented the one instance out.
• Several withRouter instances were updated. These really should have been upgraded prior to moving to v5.1 (e.g. in Update/react router 5 #15514 or Update dependency react-router-dom to v5.2.0 #15821), but better late than never.
• One instance of the now-obsolete history.listen was updated (h/t @anomiex for a solution here); see inline comment.

That last item could probably use the most scrutiny in a code review.

Other information:

• Have you written new tests for your changes, if applicable?
• Have you checked the E2E test CI results, and verified that your changes do not break them?
• Have you tested your changes on WordPress.com, if applicable (if so, you'll see a generated comment below with a script to run)?

Jetpack product discussion

Does this pull request change what data or activity we track or use?

Testing instructions:

• Verify a clickthrough works as expected.
• Verify CI is happy.

[github-actions]

Are you an Automattician? Please test your changes on all WordPress.com environments to help mitigate accidental explosions.

• To test on WoA, go to the Plugins menu on a WordPress.com Simple site. Click on the "Upload" button and follow the upgrade flow to be able to upload, install, and activate the Jetpack Beta plugin. Once the plugin is active, go to Jetpack > Jetpack Beta, select your plugin, and enable the update/jetpack/react-router-dom_v5_to_v6 branch.

  • For jetpack-mu-wpcom changes, also add define( 'JETPACK_MU_WPCOM_LOAD_VIA_BETA_PLUGIN', true ); to your wp-config.php file.

• To test on Simple, run the following command on your sandbox:

  bin/jetpack-downloader test jetpack update/jetpack/react-router-dom_v5_to_v6
  

  bin/jetpack-downloader test jetpack-mu-wpcom-plugin update/jetpack/react-router-dom_v5_to_v6
  

Interested in more tips and information?

• In your local development environment, use the jetpack rsync command to sync your changes to a WoA dev blog.
• Read more about our development workflow here: PCYsg-eg0-p2
• Figure out when your changes will be shipped to customers here: PCYsg-eg5-p2

[github-actions]

Thank you for your PR!

When contributing to Jetpack, we have a few suggestions that can help us test and review your patch:

• ✅ Include a description of your PR changes.
  
• ✅ Add a "[Status]" label (In Progress, Needs Team Review, ...).
  
• ✅ Add a "[Type]" label (Bug, Enhancement, Janitorial, Task).
  
• ✅ Add testing instructions.
  
• ✅ Specify whether this PR includes any changes to data or privacy.
  
• ✅ Add changelog entries to affected projects
  

This comment will be updated as you work on your PR and make changes. If you think that some of those checks are not needed for your PR, please explain why you think so. Thanks for cooperation 🤖

---

The e2e test report can be found here. Please note that it can take a few minutes after the e2e tests checks are complete for the report to be available.

---

Follow this PR Review Process:

1. Ensure all required checks appearing at the bottom of this PR are passing.
2. Choose a review path based on your changes:
   • A. Team Review: add the "[Status] Needs Team Review" label
     • For most changes, including minor cross-team impacts.
     • Example: Updating a team-specific component or a small change to a shared library.
   • B. Crew Review: add the "[Status] Needs Review" label
     • For significant changes to core functionality.
     • Example: Major updates to a shared library or complex features.
   • C. Both: Start with Team, then request Crew
     • For complex changes or when you need extra confidence.
     • Example: Refactor affecting multiple systems.
3. Get at least one approval before merging.

Still unsure? Reach out in #jetpack-developers for guidance!

---

Jetpack plugin:

The Jetpack plugin has different release cadences depending on the platform:

• WordPress.com Simple releases happen semi-continuously (PCYsg-Jjm-p2).
• WoA releases happen weekly.
• Releases to self-hosted sites happen monthly. The next release is scheduled for none scheduled (scheduled code freeze on undefined).

If you have any questions about the release process, please ask in the #jetpack-releases channel on Slack.

[tbradsha]

There's no direct upgrade path for this, but hopefully this roughly the same thing as before.

[anomiex]

I think I see why the E2Es are failing.

Formerly, due to lack of exact,

jetpack/projects/plugins/jetpack/_inc/client/admin.js

Lines 56 to 58 in f66d303

	<Route path="/recommendations">
	<Main routeName={ getRouteName( '/recommendations' ) } />
	</Route>

would match anything beginning with /recommendations. But that was ok, since

jetpack/projects/plugins/jetpack/_inc/client/recommendations/index.jsx

Lines 214 to 334 in f66d303

	<Switch>
	{ /* TODO: Why we don't redirect improper step paths? */ }
	<Redirect exact from={ '/recommendations' } to={ '/recommendations' + redirectPath } />
	<Route path="/recommendations/site-type">
	<SiteTypeQuestion />
	</Route>
	<Route path="/recommendations/product-suggestions">
	<ProductSuggestions />
	</Route>
	<Route path="/recommendations/product-purchased">
	<ProductPurchased />
	</Route>
	<Route path="/recommendations/agency">
	<ResourcePrompt stepSlug="agency" />
	</Route>
	<Route path="/recommendations/woocommerce">
	<FeaturePrompt stepSlug="woocommerce" />
	</Route>
	<Route path="/recommendations/monitor">
	<FeaturePrompt stepSlug="monitor" />
	</Route>
	<Route path="/recommendations/newsletter">
	<FeaturePrompt stepSlug="newsletter" />
	</Route>
	<Route path="/recommendations/related-posts">
	<FeaturePrompt stepSlug="related-posts" />
	</Route>
	<Route path="/recommendations/creative-mail">
	<FeaturePrompt stepSlug="creative-mail" />
	</Route>
	<Route path="/recommendations/site-accelerator">
	<FeaturePrompt stepSlug="site-accelerator" />
	</Route>
	<Route path="/recommendations/vaultpress-backup">
	<ResourcePrompt stepSlug="vaultpress-backup" />
	</Route>
	<Route path="/recommendations/vaultpress-for-woocommerce">
	<ResourcePrompt stepSlug="vaultpress-for-woocommerce" />
	</Route>
	<Route path="/recommendations/publicize">
	<FeaturePrompt stepSlug="publicize" isNew={ isNew( 'publicize' ) } />
	</Route>
	<Route path="/recommendations/protect">
	<FeaturePrompt stepSlug="protect" isNew={ isNew( 'protect' ) } />
	</Route>
	<Route path="/recommendations/anti-spam">
	<ResourcePrompt stepSlug="anti-spam" isNew={ isNew( 'anti-spam' ) } />
	</Route>
	<Route path="/recommendations/videopress">
	<FeaturePrompt stepSlug="videopress" isNew={ isNew( 'videopress' ) } />
	</Route>
	<Route path="/recommendations/backup-plan">
	<ResourcePrompt stepSlug="backup-plan" isNew={ isNew( 'backup-plan' ) } />
	</Route>
	<Route path="/recommendations/boost">
	<FeaturePrompt stepSlug="boost" isNew={ isNew( 'boost' ) } />
	</Route>
	<Route path="/recommendations/welcome-backup">
	<ResourcePrompt stepSlug="welcome__backup" />
	</Route>
	<Route path="/recommendations/welcome-complete">
	<ResourcePrompt stepSlug="welcome__complete" />
	</Route>
	<Route path="/recommendations/welcome-starter">
	<ResourcePrompt stepSlug="welcome__starter" />
	</Route>
	<Route path="/recommendations/welcome-security">
	<ResourcePrompt stepSlug="welcome__security" />
	</Route>
	<Route path="/recommendations/welcome-antispam">
	<ResourcePrompt stepSlug="welcome__antispam" />
	</Route>
	<Route path="/recommendations/welcome-videopress">
	<ResourcePrompt stepSlug="welcome__videopress" />
	</Route>
	<Route path="/recommendations/welcome-search">
	<ResourcePrompt stepSlug="welcome__search" />
	</Route>
	<Route path="/recommendations/welcome-scan">
	<ResourcePrompt stepSlug="welcome__scan" />
	</Route>
	<Route path="/recommendations/welcome-social-basic">
	<ResourcePrompt stepSlug="welcome__social_basic" />
	</Route>
	<Route path="/recommendations/welcome-social-v1">
	<ResourcePrompt stepSlug="welcome__social_v1" />
	</Route>
	<Route path="/recommendations/welcome-social-image-generator">
	<ResourcePrompt stepSlug="welcome__social_image_generator" />
	</Route>
	<Route path="/recommendations/welcome-golden-token">
	<ResourcePrompt stepSlug="welcome__golden_token" />
	</Route>
	<Route path="/recommendations/backup-activated">
	<ResourcePrompt stepSlug="backup-activated" />
	</Route>
	<Route path="/recommendations/scan-activated">
	<ResourcePrompt stepSlug="scan-activated" />
	</Route>
	<Route path="/recommendations/unlimited-sharing-activated">
	<ResourcePrompt stepSlug="unlimited-sharing-activated" />
	</Route>
	<Route path="/recommendations/social-v1-activated">
	<ResourcePrompt stepSlug="social-v1-activated" />
	</Route>
	<Route path="/recommendations/antispam-activated">
	<ResourcePrompt stepSlug="antispam-activated" />
	</Route>
	<Route path="/recommendations/videopress-activated">
	<ResourcePrompt stepSlug="videopress-activated" />
	</Route>
	<Route path="/recommendations/search-activated">
	<ResourcePrompt stepSlug="search-activated" />
	</Route>
	<Route path="/recommendations/server-credentials">
	<ResourcePrompt stepSlug="server-credentials" />
	</Route>
	<Route path="/recommendations/summary">
	<Summary newRecommendations={ newRecommendations } />
	</Route>
	</Switch>

uses absolute paths.

Now, though, we get the /recommandations route failing but the rest like /recommandations/site-type succeeding due to two things:

1. Since it doesn't end with /*, the route at

   jetpack/projects/plugins/jetpack/_inc/client/admin.js

   Lines 51 to 54 in bc03437

   	<Route
   	path="/recommendations"
   	element={ <Main routeName={ getRouteName( '/recommendations' ) } /> }
   	/>

   only matches /recommandations, not anything like /recommandations/site-type. The route at

   jetpack/projects/plugins/jetpack/_inc/client/recommendations/index.jsx

   Lines 216 to 219 in bc03437

   	<Route
   	path="/recommendations"
   	element={ <Navigate to={ '/recommendations' + redirectPath } replace /> }
   	/>

   fails to match it, because since paths are relative now it's trying to match for /recommandations/recommendations instead.
2. /recommandations/site-type and the like continue to work because they go via

   jetpack/projects/plugins/jetpack/_inc/client/admin.js

   Line 90 in bc03437

   <Route path="/*" element={ <Main routeName={ getRouteName( '/*' ) } /> } />

   instead.

AFAICT, to fix all this we need to make that be path="/recommendations/*" in the first bit and remove the "/recommendations" prefix from everything in the second bit (which means that first entry will become path="", and the rest can apparently be either like path="site-type" or path="/site-type").

[anomiex]

P.S. Something that's really confusing is that v6 allows absolute paths in nested routes for something like

<Routes>
  <Route path="/foo/*" element={<Foo />}>
    <Route path="/foo/bar" element={<FooBar />} />
  </Route>
</Routes>

but doesn't for a case like ours which looks more like

<Routes>
  <Route path="/foo/*" element={<Foo />} />
</Routes>

with Foo containing

<Routes>
  <Route path="/foo/bar" element={<FooBar />} />
</Routes>

So until you come across remix-run/react-router#10321 / remix-run/react-router#9841 that explicitly point this out, you're likely to find confusing results talking about it working because they only consider the first case.

[anomiex]

In manual testing, all the routes other than the recommendations stuff seem to work. 👍 Waiting on the build to test the new <Prompt> thing.

[tbradsha]

Waiting on the build to test the new <Prompt> thing.

I had a hard time figuring out where to test this, but in the end went to Settings → Discussion, toggled on "Comments", and modified the "Comment form introduction" text field, then tried to navigate away:

/wp-admin/admin.php?page=jetpack#discussion

It worked in trunk, but not in this branch...

[anomiex]

It gives me a blank screen in JN. Compiling a dev build locally, I get an error about "useBlocker must be used within a data router".

Once I jump through hoops to fix that (apparently instead of <HashRouter> it needs to use createHashRouter() and <RouterProvider>, see docs and SO), it still doesn't prompt, and I get console messages like

You are trying to use a blocker on a POP navigation to a location that was not created by @remix-run/router. This will fail silently in production. This can happen if you are navigating outside the router via window.history.pushState/window.location.hash instead of using router navigation APIs. This can also happen if you are using createHashRouter and the user manually changes the URL.

In other words, it seems borked if you use actual links for navigation. 🙄 At this point I'm inclined to punt, revert 86ffa45 and just update the comment saying we might use unstable_usePrompt if we want to bring that back but it has warnings about odd behavior and also issues with HashRouter that would need resolving.

[anomiex]

E2Es are happy, recommendations seem happy, let's do it.