Make features more declarative #4779

Or the location definitions could be functions that can be called directly, like

import repoTab from '../locations/repo-tab';

repoTab({
    append: async () => <a href={…} class="rgh-releases-tab">Releases</a>,
})

import repoSidebar from '../locations/repo-sidebar';

repoSidebar({
    update: () => {
        /* do generic stuff */
    }
})

The latter could replace most of our existing functions without too much trouble since it just takes care of the "on change" event while leaving the operations procedural.

Naive example implementation:

// locations/repo-tab.ts
const selector = '.UnderlineNav';
function repoTab({append, id}) {
    if (!isRepo()) {
        return false;
    }

    const sidebar = await elementReady(selector);
    if (!select.exists(id, sidebar) && append) {
        const element = await append();
        element.classList.add(id)
        sidebar.append(element);
    }

    const deduplicator = <rgh-repo-nav/>;
    sidebar.append(deduplicator);

    // Restart when reloaded
    onElementRemoval(deduplicator, () => repoTab({append, id}));
}

// locations/repo-sidebar
const selector = 'main .sidebar';
function repoSidebar({update}) {
    if (!isConversation()) {
        return false;
    }

    const sidebar = await elementReady(selector, {children: false});
    update();
    const deduplicator = <rgh-repo-sidebar/>;
    sidebar.append(deduplicator);

    // Restart when reloaded
    onElementRemoval(deduplicator, () => repoSidebar({update}));
}

This would basically move a lot of the logic from the feature files to the locations. Probably this won't reduce the code, but it would make it easier to group behavior by location rather than by feature, so that multiple features (if they exist), can reuse the logic

For example location: reactions would mean:

• hasComments -> onNewComments -> update
• isReleaseOrTag/isDiscussion -> MutationObserver -> update

They could also be nested, for example location: comment-dropdown would mean:

• location: comment -> await dropdown opening -> update

Unfortunately ain't nobody got time for that. Such a system would require a lot of research work and trial and error, as well as rewriting existing features and the system itself as we regularly reach its design limits.

I think selector-observer already gets us 50% there if used correctly. We can start migrating to using that exclusively instead of using custom events (unless they're really simple).

The only issue is that it awaits DOM ready, but for many features that's not an issue, and we could look for an alternative library without such limitation if necessary.

Unfortunately ain't nobody got time for that.

Why not just keep the issue open for a bit longer then?

That said, there indeed isn't a huge need to design a full-blown system and rewrite most features since few features target the same component/location, which means we don't have many chance to reduce duplicate logic.

The repo-tab example you provide is valid though, maybe it's useful to make a helper for that.

I just don't think it's worth going too far off the current loading setup if we don't plan on going all in. I don't want to have just a few features that handle loading completely differently.

I'm not a fan of keeping "Let's rewrite the core" issues open because nobody except current maintainers will pick them up. We can still discuss it though and exploratory PRs can still be opened.

Generally though I think that there aren't many features that can be generalized this way. Sometimes we have nested ajaxed elements so even if I could target repo-sidebar nothing is still stopping GitHub from adding an ajaxed component inside and breaking all of our deduplication logic. Only specific selector-observer calls really can do that.

If there are reusable pieces, we can wrap whatever mechanism we have in functions like we're already doing: https://github.com/sindresorhus/refined-github/blob/main/source/github-events/on-pr-merge-panel-open.ts