pinoceniccola / what-hn-says-webext

Web Extension: Easily find Hacker News discussions about the page you're currently browsing.
MIT License
266 stars 13 forks source link

Clean up analytics-related query params from URL for better search results #7

Closed memoryonrepeat closed 3 years ago

memoryonrepeat commented 3 years ago

As discussed here

I'm not sure about the www case - the extension seems to either work with / without it, so I kept it intact.

pinoceniccola commented 3 years ago

The query regex seems to leave a question mark (?) at the end of the url. Also, the & sign when there are many query parameters, e.g.:

http://example.com/?utm_test=1 > example.com/? http://example.com/?test=1&gclid=1 > example.com/?test=1&

Besides, it would be nice to have an array of query keys to strip where you can add more keys later in case of need, something like:

const queryBlacklist = ['utm_',  'clid'];

Perhaps this may be a lot more simpler with the new URL and URLSearchParams APIs instead of regex (code not tested, just the idea):

// Create a URLSearchParams object
const params = new URL(url).searchParams;

// Iterate keys
for (let key of params.keys()) {
  // ...
  // if key matches one of the queryBlacklist keys, then just...
    params.delete(key):
}

But hey, your solution is fine too, please just fix the cases above.

And one last thing, the single leading slash rule should be the last one, otherwise it won't work.

Thanks!

memoryonrepeat commented 3 years ago

Yes I'm aware of the leftover ? case, but I decided to leave it there because there might still be other necessary parameters in the URL that should not be cleaned up.

But I do like the URL API approach as it's more maintainable. Will push an update later using that. Thanks for reviewing.

memoryonrepeat commented 3 years ago

Hey @pinoceniccola I have pushed the new version ☝️

pinoceniccola commented 3 years ago

Looks great, will push the updated version later to the stores.

Thanks.