Ability to remotely disable broken features #3529

I would say the points under the 'Notes' section should be the Acceptance Criteria.

Also, it would make sense to make it a 'feature' so that users can opt-in for these hotfixes. If not, they are at their own risk.

This could possibly go into the "Debugging" section in the Preferences tab of the extension.

I have not developed on a WebExtension before, but I'm willing to try. Is this 'good-first' enough for a person like me?

Also, it would make sense to make it a 'feature' so that users can opt-in for these hotfixes. If not, they are at their own risk.

No, it would not make sense. This solution is meant to be instant and for the user’s own good.

Is this 'good-first' enough for a person like me?

That'd be great! You can try following contributing.md and keep it simple. I think the code is not far from:

const fetchHotfixesAsynchronously = cache.function(async () => {
    const response = await fetch('https://raw.githubusercontent.com/sindresorhus/refined-github/hotfix/hotfix.json');
    const hofixes = await response.json();
    if (!hotfixes) {
        return {};
    }

    if (hotfixes.unaffected) {
        const currentVersion = await browser.etc get version()
        if (looseVersionCompare(hotfixes.unaffected, currentGetVersion) >= 0) {
            return {};
        }
    }

    return hotfixes;
}, {
    maxAge: {hourse: 6},
    key: 'hotfix'
});

And we can change this file: https://github.com/sindresorhus/refined-github/blob/hotfix/hotfix.json

If there are no hotfixes, the content will be false, which AFAIK is valid JSON

Great, I'm going to try this and see how close I can get. Thank you for the code example. That is a head-start.

As for the files, do we need to introduce a new directory in the tree for one file? I don't see anything that could expand into that directory. I'm thinking along the lines of source/hotfix.ts and source/hotfix.json. Does that make sense?

straying away from the KIS philosophy, I'm going to ask this question : what if, somehow, my browser did not update my extension to an offending version but the hotfix poll would disable the feature for me anyway?

also, how often do we keep updating hotfix.json? I mean, how long does a hotfix version stay in the hotfix.json file before it is deemed safe to be cleared?

because I'm ignoring KIS philosophy for this comment, I want to elaborate and include a first and last tag to hotfix.json that looks like :

{
    "cleanup-repo-filelist-actions": {
        "first": "20.9.3",
        "last": "20.9.4"
    }
}

that indicates when a bug was introduced and when it was fixed. the version compare logic should apply to the versions in between.

I guess this can be the next iteration, let me see if I can get the first iteration working in the first place.

LooseVersionCompare would tell you if your current version is after the first unaffected

Another thing to keep in mind that it should not disable debug versions otherwise it will be impossible to fix.

As for the files, do we need to introduce a new directory in the tree for one file?

No we can just keep exactly this file on the hotfix branch: https://github.com/sindresorhus/refined-github/blob/hotfix/hotfix.json

So we can change it at will without making changes on master.

straying away from the KIS philosophy

Let's not. 😁

also, how often do we keep updating hotfix.json

This happens once or twice a year and the file is only meant to last less than a week to ensure everyone has updated, so it doesn't need to be that complicated at all. This solution is only to cover extreme breakage until a new version is pushed and downloaded (which already has its own mechanism to disable broken features). After 7 days the file will likely be reset to false. This is not meant to be a perennial list of broken features.

include a first and last tag

If some user on an older, unaffected version of RG loses access to feature xyz for 7 days, it's not a big deal. Refined GitHub is progressive enhancement; the lack of (part of) it is not a deal-breaker.

Another thing to keep in mind that it should not disable debug versions otherwise it will be impossible to fix.

Good point, in our case it that means:

const hotfix = chrome.manifest.version === '0.0.0' || await cache.get('hotfix');

Edit: the code should be exactly this. Firefox reuses the same options when adding the development version, so we cannot rely on or affect the cache.

on the hotfix branch

this was the only fact that I missed to notice. Though it was the most important fact to not miss. :facepalm:

If it is not on master, then none of my arguments really matter at all.

I'm going to put some extra attention and read all the comments on this issue more carefully so that I don't miss any more of the direct instructions like this.

One last piece of code would be also run cache.delete('hotfix') when the extension is updated, which is this listener: https://github.com/sindresorhus/refined-github/blob/e1d836a2f542bb77e9510e5c5aaed2af8c572c91/source/background.ts#L24

I am kind of lost at looseVersionCompare.

1. Is that a function that is already present somewhere in the codebase?
2. Is it a package I need to add?
3. Is it an export of the tiny-version-compare package that is already included as a dependency?

My answer to all of those questions after digging into the codebase is 'no'.

1. git grep --ignore-case looseVersionCompare yielded no results
2. I looked for loose version compare on npmjs.com in vain. However, I found a ( 2-year-old ) version of a package on an aliyun npm mirror that apparently seems like a predecessor of tiny-version-compare.
3. I dug into the source of tiny-version-compare and could not find an export called looseVersionCompare. There is only a single function export. So does that mean I should import looseVersionCompare from tiny-version-compare?

Also, tiny-version-compare is currently being imported only in source/features/tag-changelog-link.tsx and source/github-helpers/index.ts ( with 2 different names; because function export ) at the moment

I need some pointers here.

I also made some changes to the pseudo-patch provided in https://github.com/sindresorhus/refined-github/issues/3529#issuecomment-687367554. I'm putting them here to validate if I'm on the right track.

-        const currentVersion = await browser.etc get version()
+        const currentVersion = await browser.extension get version()

-        if (looseVersionCompare(hotfixes.unaffected, currentGetVersion) >= 0) {
+        if (looseVersionCompare(hotfixes.unaffected, currentVersion) >= 0) {

-    maxAge: {hourse: 6},
+    maxAge: {hours: 6},

I made 2 assumptions and opened #3530 :

1. import looseVersionCompare from tiny-version-compare
2. all the other changes that I mentioned that I made in https://github.com/sindresorhus/refined-github/issues/3529#issuecomment-687473518 are correct.

If I am wrong, please point them out on the pull request review.

I learnt the hard way that browser.etc get version() meant find the corresponding browser API that provides the installed extension's version. The tests and build failed on me. I didn't think of it earlier, but I eventually found browser.runtime.getManifest().version.

I am kind of lost at looseVersionCompare.

Sorry about that! It's my own module and I even forgot how I named it 😅

Everything else is about right. I wrote that code in the comment box to explain clearly what I had in mind; expect typos and subtle exceptions because I didn't run it.