Skip to content

fix(URLPattern): Handle search parameters correctly in URLPattern with base#49

Closed
yazan-abdalrahman wants to merge 2 commits into
denoland:mainfrom
yazan-abdalrahman:fix(URLPattern)-Handle-search-parameters-correctly-in-URLPattern-with-base
Closed

fix(URLPattern): Handle search parameters correctly in URLPattern with base#49
yazan-abdalrahman wants to merge 2 commits into
denoland:mainfrom
yazan-abdalrahman:fix(URLPattern)-Handle-search-parameters-correctly-in-URLPattern-with-base

Conversation

@yazan-abdalrahman
Copy link
Copy Markdown

…parameters when a base URL is provided. This fixes the discrepancy where URLPattern in Deno was returning null for URLs with search parameters, unlike the behavior observed in Chrome.

Adjusted URLPattern parsing to include search parameters when matching against base URLs. Fixes #denoland/deno#24266

…parameters when a base URL is provided. This fixes the discrepancy where URLPattern in Deno was returning null for URLs with search parameters, unlike the behavior observed in Chrome.

Adjusted URLPattern parsing to include search parameters when matching against base URLs.
Fixes #denoland/deno#24266
@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Jul 22, 2024

CLA assistant check
All committers have signed the CLA.

@yazan-abdalrahman
Copy link
Copy Markdown
Author

@crowlKats
Hello,

As I previously mentioned, I found that when the "URLPattern" is constructed in Deno, it attempts to establish the pattern; however, if no pattern is specified, the default pattern for the "search field" is set to None("")

so in 'p.exec' , it attempts to extract patterns and compares them with those generated by the constructor.
so if there is no match, it will return NULL

check out this example, which attempts to match ?id=2 with ?id=3 ,the result no match, null was returned.
image

and check out this example, it tries to match ?id=3 with ?id=3  ,the result matched, The object was returned
image

So, if no pattern was specified, the search pattern will be set to NONE, meaning there would be no match and no result.
image

So I wrote change to set the search pattern default value to .
'
' indicates that it will fit any pattern.
image


I have an additional fix. I like it better; it's more like Chrome behavior, but I'll have to rewrite every test in the "url-pattern" from start if you want this change. It will take me a few days to complete the rewrites.

image
image
image

so if we apply this change, we can remove our construct code of UrlPattern Init ,this is simpler and has fewer difficult code snippets.
but as it changes the default value of pattern for every field, its need to  rewrite  tests from  scratch.

@yazan-abdalrahman
Copy link
Copy Markdown
Author

@crowlKats
Please see the lint. It failed in line I didn't write it.

@crowlKats
Copy link
Copy Markdown
Member

crowlKats commented Jul 22, 2024

@yazan-abdalrahman the tests in the JSON file come from WPT, which are not to be changed since they match the standard specification.
I'll tkae a deeper look at this PR later today or tomorrow.

@yazan-abdalrahman
Copy link
Copy Markdown
Author

@crowlKats
Hello ,it would be better to reimplement the "URL Pattern" constructor to be depends on the URL. With regard to the update test in "wtp," I don't think its tandard specification. i think it should be changed because the result depends on the generated pattern when creating the "URL Pattern," so changing the default pattern will need to change the test.

#50
However, this is a workaround for the issue and no tests changed.

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