Switch from @require to @match

[arsinclair]

The linter reports that:

Using @include is potentially unsafe and may be obsolete in Manifest v3. Please switch to @match

Using @match directives is considered more safe, however they don't support Regex, only globbing.

So for bandcamp importer, it would be something like this

--- // @include      /^https:\/\/[^/]+\/(?:(?:(?:album|track))\/[^/]+|music)$/
--- // @include      /^https:\/\/([^.]+)\.bandcamp\.com((?:\/(?:(?:album|track))\/[^/]+|\/|\/music)?)$/
--- // @include      /^https?:\/\/web\.archive\.org\/web\/\d+\/https?:\/\/[^/]+(?:\/(?:album|track)\/[^/]+\/?|\/music\/?|\/?)$/
+++ // @match        https://*/*/album/*
+++ // @match        https://*/*/track/*
+++ // @match        https://*/*/music*
+++ // @match        https://*.bandcamp.com/album/*
+++ // @match        https://*.bandcamp.com/track/*
+++ // @match        https://*.bandcamp.com/music*
+++ // @match        https://*.bandcamp.com/
+++ // @match        https://web.archive.org/web/*/https://*/album/*
+++ // @match        https://web.archive.org/web/*/https://*/track/*
+++ // @match        https://web.archive.org/web/*/https://*/music*
+++ // @match        https://web.archive.org/web/*/https://*/

Question: should we move towards matching or should we silence the eslint warning, until (if) Manifest v3 removes the support? What are your thoughts?

[murdos]

I don't have a strong opinion on the subject.
I introduced the linter to have some guidance on good practices, but it's ok to disable some rules if we don't want to follow them.
On the other hand, even if the match rules are a bit more verbose, I find them clearer and maybe easier to maintain.

So I trust you for the decision 🙂

[arsinclair]

I am not sure myself, but I think I have a number of arguments to support staying with @include for now.

1. Although, it's better to stay more aligned with the spec, and Manifest V3 is definitely not going away: they haven't discontinued @include yet and I'm not seeing any plans so far.
2. @match is less versatile than @include, it doesn't support regex and because of that it limits us in what we can actually do with it. Sometimes Regex allows us to express even more strict matching patterns, so abandoning it feels like a step back.
3. After some reading into it, it seems one of the reasons they wanted to discontinue @include is because it has a pontential security vulnerability: an attacker could craft a URL in a certain way, making the script run on their domain as well, even if the @include didn't explicitly allow it. For us this is not a problem, since all our scripts are stateless scripts, created with the sole purpose of either improving the UI of a certain platform, or facilitating the import of the data to MB. If an attacker can manage to run the script on a 3rd party domain that doesn't threaten us in any way.
4. The amount of testing we will have to do after switching every single script to @match is not small, and I'm not looking forward to it, and also some bugs might still be introduced in the process.
5. There's no immediate win from switching to @match, we're not gaining anything at all.

I agree that @match is more verbose and I like it too, but I'm not sure how well will it play with the scripts that need to match a lot of things in a complex way, such as bandcamp importer. Thankfully we don't have a lot of them, only 1 or 2.

That is to say, I fully understand how futile this switch at the moment will be, but I'm still looking for reasons to do it, I guess. I'll give it a week, and if there's no more arguments shared meanwhile in support or against the move, I can start migrating all scripts to @match incrementally.