Skip to content

Conversation

edgul
Copy link
Contributor

@edgul edgul commented Jul 31, 2025

URLPattern crate doesn't seem to test for quirks::parse_pattern but one of the failing tests when invoked from firefox

{
    "pattern": [{ "pathname": "/(\\m)" }],
    "expected_obj": "error"
  },

Relevant spec location: https://urlpattern.spec.whatwg.org/#compile-a-component
I'm not seeing the place in the algorithm where we should explicitly run the parser eval, but the spec notes the following:
"
The specification uses regular expressions to perform all matching, but this is not mandated. Implementations are free to perform matching directly against the part list when possible; e.g. when there are no custom regexp matching groups. If there are custom regular expressions, however, its important that they be immediately evaluated in the compile a component algorithm so an error can be thrown if they are invalid.
"

This is a bit of a hack and probably we should just be feeding Spidermonkey regex into the pattern, but it seems to fix the test without breaking anything else.

@edgul edgul force-pushed the wpt-slash-slash-m branch from 00dccf7 to da87019 Compare July 31, 2025 21:06
@edgul edgul force-pushed the wpt-slash-slash-m branch from da87019 to aff4bd1 Compare July 31, 2025 21:11
@edgul
Copy link
Contributor Author

edgul commented Aug 6, 2025

Hey @lucacasonato, I'm not able to add a reviewer, can you take a look?

@valenting
Copy link
Contributor

@lucacasonato @crowlKats friendly ping.
Is there anything we can do to expedite these PRs? They are a blocker for urlpattern interop

Copy link
Member

@crowlKats crowlKats left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM; I think this is fine as-is even if it is a bit hacky.

@crowlKats crowlKats merged commit c0988c3 into denoland:main Aug 20, 2025
2 checks passed
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

Successfully merging this pull request may close these issues.

3 participants