Skip to content

fix: metadata @match now for url path plus query string#463

Merged
ACTCD merged 2 commits into
masterfrom
issue/454
Apr 14, 2023
Merged

fix: metadata @match now for url path plus query string#463
ACTCD merged 2 commits into
masterfrom
issue/454

Conversation

@ACTCD

@ACTCD ACTCD commented Apr 12, 2023

Copy link
Copy Markdown
Collaborator

According to the MDN docs on the definition of path in Match patterns:

The value for the path matches against the string which is the URL path plus the URL query string. This includes the ? between the two, if the query string is present in the URL. For example, if you want to match URLs on any domain where the URL path ends with foo.bar, then you need to use an array of Match Patterns like ['*://*/*foo.bar', '*://*/*foo.bar?*']. The ?* is needed, rather than just bar*, in order to anchor the ending * as applying to the URL query string and not some portion of the URL path.

fixed #454

@ACTCD ACTCD linked an issue Apr 12, 2023 that may be closed by this pull request
@ACTCD
ACTCD requested a review from quoid April 12, 2023 11:50

@quoid quoid left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Looks good but this test is now failing:

I think that test should be updated with new cases that include query strings that should both pass and fail.

@ACTCD

ACTCD commented Apr 13, 2023

Copy link
Copy Markdown
Collaborator Author

@quoid
I forgot to check other swift files for calls to this func. And I never really got to know XCTest, could you please update it?

@ACTCD
ACTCD requested a review from quoid April 13, 2023 11:07
@ACTCD

ACTCD commented Apr 13, 2023

Copy link
Copy Markdown
Collaborator Author

Now tests have been fixed and new cases added.

@quoid quoid left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Great work, thank you 🚀

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.

@match with query parameters not working

2 participants