Fix inadvertent support for partial dynamic parameters

[brophdawg11]

The intention was never to match partial dynamic or splat parameters, but we had some bugs and inconsistencies in the implementation:

• matchPath would not match partial splat parameters (/prefix*), and would warn + treat them as /prefix/*
• However, generatePath would match partial splat parameters
• Both matchPath/generatePath would match partial named parameters (/prefix:param)

This PR removes the buggy support for partials matching such that we always require dynamic/splat params to be an entire URL segment. This will simplify our future plans to support optional params as well as the intended optimization of the path-matching algorithm.

If an application happened to be relying on this, then the suggestion should be to extract the static prefix at the useParams call-site:

// Old behavior at URL /prefix-123
<Route path="prefix-:id" element={<Comp /> }>

function Comp() {
  let params = useParams(); // { id: '123' }
  let id = params.id; // "123"
  ...
}

// New behavior at URL /prefix-123
<Route path=":id" element={<Comp /> }>

function Comp() {
  let params = useParams(); // { id: 'prefix-123' }
  let id = params.id.replace(/^prefix-/, ''); // "123"
  ...
}

Replaces #9238

[changeset-bot]

🦋 Changeset detected

Latest commit: db7a8e6

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	Patch
@remix-run/router	Patch
react-router-dom	Patch
react-router-dom-v5-compat	Patch
react-router-native	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

[brophdawg11]

The lack of search/hash in this test case was causing weird console.warn of /threeundefinedundefined

[brophdawg11]

Same logic as we use in compilePath for partial splats

[brophdawg11]

generatePath does 2 separate checks here to avoid a nasty regex to differentiate :param from /thing/:param

[brophdawg11]

Only match params that are at the start of a URL segment. We do not need to handle ^: here since line 693 ensures our regexpSource always has a leading slash by this point. So we just only collect params when they come immediately following a slash

[pcattori]

For matching types in TS, I think it'd be good to replace Path extends `${string}:${infer Param}` with Path extends `:${infer Param)` (so not expecting the leading string before :) in the implementation of _PathParam

[mjackson]

LGTM. Thanks, @brophdawg11

[brophdawg11]

Thanks @pcattori - that looks like it works: https://www.typescriptlang.org/play?#code/PTAECUFMGMFcCcDOBLAbpUALSAbADpPKAGYD2RxyAdgCbUDmoeAhgC6ZPPzMC2krhRKGqh2GZgCNEkKtAylioAO7IcNaFxqIAUKwCeBUAH0ACm0xnuPADxn2oSAA8BtIYlbwGAPlABebaCgIKCIeDjIrJz21KykwrRoyDSwzDhRHNL0fFSsOoF2HE4uWqAABgAkAN7UxISgADIAvsBVNXXgjaUBgaAA-MYFlrzW9T4APgPmQzbgXt2BAFxBYJS0nFZCzMQCRKULXT2gBQ7OMiV7rVS1RNOd831HXLz3S8GkYvAq0gA0otjw4gBVAA5JFmFQ9OteEI8ADpDl7oEqJB0PAANzabQ4figRxLUxTJ42ABEwGwOBwpGACzIVIWEi4wFhkEojgAtPTmAAvYAAKmJXiAA

[YousefED]

@brophdawg11 I'm looking at your suggested workaround for when this functionality is required, but I don't understand how the workaround would work with the rest of the Routing system.

E.g.: I want to "route" requests to /@<username> to ProfilePage

According to your workaround, I need to create a route for /:username. But then it would catch everything. How do I tie it in to the rest of the routing system, e.g.: I would like a request to /undefined-url result in a 404 catch all handler

[brophdawg11]

If you're using RouterProvider, you can detect and throw in the loader to render the errorElement:

let router = createBrowserRouter([{
  path: ":username",
  loader({ params }) {
    if (!params.username.startsWith('@')) {
      throw new Response("Not Found", { status: 404 });
    }
    let user = await getUser(params.username);
    return { user };
  },
  element: <ProfilePage />,
  errorElement: <NotFound />
}]);

If you're using <BrowserRouter> then you can conditionally render in your element:

// <Route path=":username" element={<GuardedProfilePage />} >

function GuardedProfilePage() {
  let params = useParams();
  if (!params.username.startsWith("@")) {
    return <NotFound />
  }

  return <ProfilePage username={params.username} />
}

[YousefED]

@brophdawg11 Thanks.

The use-case I'd like to solve is this:

<Route path="@:userParam" element={<ProfilePage/>} />
<Route path="*" element={<Fallback>} />

Now, this is currently not possible, because @:userParam is invalid.

So the next step would be:

<Route path=":userParam" element={<ProfilePage/>} />
<Route path="*" element={<Fallback>} />

This would mean Fallback never gets triggered. So I either need to put the Profile route within the Fallback, or the other way around. It's doable, but imo it couples unrelated components of my app together

[brophdawg11]

The second example above is what you want, instead of splitting that logic across two ambiguous routes, you just fork in the :username element when the username is not the pattern you want. You don't need to touch your current implementations of ProfilePage or Fallback.

// <Route path=":userParam" element={<ProfilePageElement />} />

function ProfilePageElement() {
  let { userParam } = useParams();
  return userParam.startsWith("@") ? <ProfilePage /> : <Fallback>;
}

[dazlious]

I understand that this PR is not related to semantic versioning, but how come that this was not considered a breaking change? IMO this breaks a lot of applications.

I stumbled upon this tiny "patch" while trying to upgrade from 6.4.2 to 6.5.0. In the patchnotes this PR is linked under patches.

Our use case ist the following structure:

https://www.foo.bar/:locale-:country/further/path/here/

[pcattori]

Hi @dazlious , sorry that you had a bad experience with this.

The main issue we were facing was that multiple params were not spec'd out and were extremely inconsistent in how they behaved depending on your path segments. In fact, some users had similar setups where the second : was treated as a literal and they relied on this, knowing that our docs/tests/specs all said this was the intended behavior.

Unfortunately, this is one of those cases where a bug opens the door to a (brittle, undocumented) feature and a "bug fix" for some becomes a "breaking change" for others. I think you are right that we should call this out as a breaking change anyway since anyone not relying on it would just skip over that.

Going to bring this up with the team and see if we can promote this to a breaking change in the release notes.