Skip to content
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

URLPattern {/:optional}? no longer works as expected. #2693

Closed
JTRNS opened this issue Oct 6, 2024 · 1 comment
Closed

URLPattern {/:optional}? no longer works as expected. #2693

JTRNS opened this issue Oct 6, 2024 · 1 comment

Comments

@JTRNS
Copy link

JTRNS commented Oct 6, 2024

TLDR

const pattern = new URLPattern({pathname: "/books{/:genre}?" });
console.log(pattern.pathname) // PREVIOUSLY resulted in "/books{/:genre}?"
console.log(pattern.pathname) // CURRENTLY results in "/books/:genre?"

Just wanted to give you a heads up on some additional URLPattern changes that impact the current router logic inside the pathToPattern function.

// snippet from /src/router.ts
if (part[j + 1] === "[") {
  // Disallow optional dynamic params like `foo-[[bar]]`
  if (part[j - 1] !== "/" && !!part[j - 1]) {
    throw new SyntaxError(
      `Invalid route pattern: "${path}". An optional parameter needs to be a full segment.`,
    );
  }
  groupOpen++;
  optional = true;
  pattern += "{/";
  j++;
}

Haven't exactly been able to figure out why, but I did notice that the test case for this pattern was recently removed from the deno repository.

I am assuming it simply didn't comply with the specification, but not completely sure. If you have some more insight into the why this URL pattern is no longer supported I'd love to know as I was using it too.

@marvinhagemeister
Copy link
Collaborator

There was a recent specification change in URLPattern, see whatwg/urlpattern#198 . It allows group values to be undefined instead of always falling back to an empty string.

Taking your pattern as an example: The genre group can now be undefined when passing http://example.com/books without a genre.

// Before
> new URLPattern({pathname: "/books{/:genre}?" }).exec("https://foo.com/books")
{
  inputs: [ "https://foo.com/books" ],
  protocol: { input: "https", groups: { "0": "https" } },
  username: { input: "", groups: { "0": "" } },
  password: { input: "", groups: { "0": "" } },
  hostname: { input: "foo.com", groups: { "0": "foo.com" } },
  port: { input: "", groups: { "0": "" } },
  pathname: { input: "/books", groups: { genre: "" } },   //  <-- falls back to empty string
  search: { input: "", groups: { "0": "" } },
  hash: { input: "", groups: { "0": "" } }
}

// After
> new URLPattern({pathname: "/books{/:genre}?" }).exec("https://foo.com/books")
{
  inputs: [ "https://foo.com/books" ],
  protocol: { input: "https", groups: { "0": "https" } },
  username: { input: "", groups: { "0": "" } },
  password: { input: "", groups: { "0": "" } },
  hostname: { input: "foo.com", groups: { "0": "foo.com" } },
  port: { input: "", groups: { "0": "" } },
  pathname: { input: "/books", groups: { genre: undefined } },   //  <-- is undefined
  search: { input: "", groups: { "0": "" } },
  hash: { input: "", groups: { "0": "" } }
}

In Fresh we always normalize it to a string for now to avoid introducing a breaking change to users.

@JTRNS JTRNS closed this as completed Oct 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants