Skip to content

Conversation

joseph0926
Copy link

@joseph0926 joseph0926 commented Sep 1, 2025

Fix generatePath to handle parameters with static suffixes

Fixes #14040

Problem

The current regex /^:([\w-]+)(\??)$/ in generatePath requires an exact match of the entire segment to be a parameter. The $ anchor means the segment must end immediately after the parameter name, causing patterns like :id.json to fail.

In v6.8.2 this worked correctly, but broke in v6.9.0.
And this issue also exists in v7.
You can test it at the StackBlitz link provided.

Solution

Modified the regex to /^:([\w-]+)(\??)(.*)/ which

  • Removes the $ anchor
  • Captures any remaining text after the parameter as a suffix
  • Appends the suffix to the interpolated result

Changes

- const keyMatch = segment.match(/^:([\w-]+)(\??)$/);
+ const keyMatch = segment.match(/^:([\w-]+)(\??)(.*)/);
  if (keyMatch) {
-   const [, key, optional] = keyMatch;
+   const [, key, optional, suffix] = keyMatch;
    let param = params[key as PathParam<Path>];
    invariant(optional === "?" || param != null, `Missing ":${key}" param`);
-   return encodeURIComponent(stringify(param));
+   return encodeURIComponent(stringify(param)) + suffix;
  }

Tests

Added comprehensive tests for parameters with static text

describe("with params followed by static text", () => {
  it("interpolates params with file extensions", () => {
    expect(generatePath("/books/:id.json", { id: "42" })).toBe(
      "/books/42.json",
    );
    expect(generatePath("/api/:resource.xml", { resource: "users" })).toBe(
      "/api/users.xml",
    );
    expect(generatePath("/:lang.html", { lang: "en" })).toBe("/en.html");
  });

  it("handles multiple extensions", () => {
    expect(generatePath("/files/:name.tar.gz", { name: "archive" })).toBe(
      "/files/archive.tar.gz",
    );
    expect(generatePath("/:file.min.js", { file: "app" })).toBe(
      "/app.min.js",
    );
  });
});

All existing tests continue to pass, confirming no breaking changes.

Copy link

changeset-bot bot commented Sep 1, 2025

⚠️ No Changeset found

Latest commit: d70c8a4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@timdorr
Copy link
Member

timdorr commented Sep 1, 2025

This PR is against the dev branch, which is the 7.x version of the code. I think you meant to base this against v6, which is for the 6.x version.

@joseph0926
Copy link
Author

joseph0926 commented Sep 1, 2025

This PR is against the dev branch, which is the 7.x version of the code. I think you meant to base this against v6, which is for the 6.x version.

@timdorr
Thanks for the review! You're correct about the branch targeting.

I've created a reproduction showing this affects both versions

  • v7 (dev branch): StackBlitz => Clicking “Test Generate Path” and navigating to that router will allow you to confirm that the issue is reproduced.
  • v6.9.0+: Regression from v6.8.2 (original issue)

The fix is identical for both versions. Should I

  1. Retarget this PR to v6 for the regression fix?
  2. Keep for v7 and create a separate v6 PR?
  3. Or would you prefer to handle the backport?

Happy to proceed however works best for your workflow.

@joseph0926
Copy link
Author

This PR fixes the same issue as the original PR, but targets a fix for v6.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants