Regression of #13595 - URLs with optional segments should still be included non-expanded in typed href helper if they are unambiguous (no static optional segments) by the supplied params object

[tobimori]

I'm using React Router as a...

framework

Reproduction

See Expected Behavior for Details

System Info

RR 7.6.1

Used Package Manager

pnpm

Expected Behavior

I'm posting this as a bug report since this working behavior was removed/regressed in a minor release, which breaks a few of our sites.

For example, in a lot of our projects we have multiple optional segments that are dynamic. We then use the href helper everywhere to generate correct URLs, and we always supply the params object from ComponentProps.

For example:

href('/:segment?/xxx', params) // params could either be { } or { segment: 'something' }

In this case, the inclusion of the params in the object is optional - it perfectly suited the case.

Now, with the new syntax, I'd have to implement the type checking by myself, the helper is now basically useless:

if("segment" in params) {
   return href('/:segment/xxx', params) 
} else {
   return href('/xxx', params) 
}

(This would grow large in case you have more optional params, we have like 5 in the largest URL, so this would be quite cumbersome to work around.)

Actual Behavior

There's a type error shown for all links since the working behavior was removed in RR 7.6.1.

Ref: #13595

[tobimori]

What would be perfect would be a merge between 7.6.0 and 7.6.1 behavior:

/:localeOrLocation?/:location?/static?

would turn into

(add these two)
href('/:localeOrLocation?/:location?', params)
href('/:localeOrLocation?/:location?/static', params)

(already exist in current version)
href('/')
href('/:localeOrLocation/:location', params)
href('/:localeOrLocation', params)
href('/static')
href('/:localeOrLocation/:location/static', params)
href('/:localeOrLocation/static', params)

[tobimori]

I don't really have the time to submit a PR and get known with the contributing process, but here's a TS Play with a method that could replace the exploding method.
(Click run and check logs)

function explodeOptionalSegments(path: string): string[] {
  let segments = path.split("/");
  if (segments.length === 0) return [];

  let [first, ...rest] = segments;
  
  // Optional path segments are denoted by a trailing `?`
  let isOptional = first.endsWith("?");

  // Compute the corresponding required segment: `foo?` -> `foo`
  let required = first.replace(/\?$/, "");
  
  // Check if this is a dynamic parameter (starts with :)
  let isDynamic = required.startsWith(":");
  
  if (rest.length === 0) {
    // Interpret empty string as omitting an optional segment
    // `["one", "", "three"]` corresponds to omitting `:two` from `/one/:two?/three` -> `/one/three`
    // If it's an optional dynamic param, also include the version with ?
    if (isOptional) {
      let results = [required, ""];
      if (isDynamic) {
        results.push(first); // Include the version with ? for dynamic params
      }

      return results;
    }

    return [required];
  }

  let restExploded = explodeOptionalSegments(rest.join("/"));

  let result: string[] = [];

  // All child paths with the prefix.  Do this for all children before the
  // optional version for all children, so we get consistent ordering where the
  // parent optional aspect is preferred as required.  Otherwise, we can get
  // child sections interspersed where deeper optional segments are higher than
  // parent optional segments, where for example, /:two would explode *earlier*
  // then /:one.  By always including the parent as required *for all children*
  // first, we avoid this issue
  result.push(
    ...restExploded.map((subpath) =>
      subpath === "" ? required : [required, subpath].join("/")
    )
  );
  
  // Then, if this is an optional value, add all child versions without
  if (isOptional) {
    result.push(...restExploded);
    
    // If this is an optional dynamic param, also add versions where it stays optional
    if (isDynamic) {
      result.push(
        ...restExploded.map((subpath) =>
          subpath === "" ? first : [first, subpath].join("/")
        )
      );
    }
  }

  // for absolute paths, ensure `/` instead of empty segment
  return result.map((exploded) =>
    path.startsWith("/") && exploded === "" ? "/" : exploded
  );
}

console.log(explodeOptionalSegments('/:localeOrLocation?/:location?/static?'))
console.log(explodeOptionalSegments('/test?/blabla?/static?'))
console.log(explodeOptionalSegments('/:reallyDynamic?/:dynamic?/static?'))

Feel free to use this for fixing this.

This is more or less a stylistic choice, but the code above would create the following combos:

:localeOrLocation?/:location?
:localeOrLocation/:location?
:localeOrLocation?/:location

It might make sense to only create the fully optional version in case we have multiple dynamic segments. But I'll let that over to you.

[tobimori]

you could also only expand static optional segments (would be the most clean implementation) but that would break anyone on version 7.6.1 that now starts to use the typegen route (only on a typelevel, runtime is unaffected)

[pcattori]

@tobimori thanks for all the investigation you did here! I agree that a key part of what makes href useful is the ability to pass in params without needing to do your own type-narrowing and view this as a regression / breaking change in 7.6.1 . To that end, I think its best to revert this behavior and then fix it by expanding only static optional segments like you suggested.

[pcattori]

Fixed by #13725 which will be available next release

[github-actions]

🤖 Hello there,

We just published version v0.0.0-nightly-3ab810de1-20250603 which involves this issue. If you'd like to take it for a test run please try it out and let us know what you think!

Thanks!