Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

latest-tag-button sometimes fails getting the "ahead-by count" #2927

Closed
fregante opened this issue Mar 25, 2020 · 6 comments · Fixed by #3199
Closed

latest-tag-button sometimes fails getting the "ahead-by count" #2927

fregante opened this issue Mar 25, 2020 · 6 comments · Fixed by #3199

Comments

@fregante
Copy link
Member

https://github.com/sindresorhus/refined-github/blob/c84162f1dc7ad898c7a3e303da285004c8c996da/source/features/latest-tag-button.tsx#L73-L74

The expected text is missing from that page: https://github.com/fregante/webext-storage-cache/releases/tag/v2.3.1)

Sadly the count was quite difficult to get. Here was the previous attempt via API: #2771 (comment)

Example

https://github.com/fregante/webext-storage-cache

example

@yakov116
Copy link
Member

yakov116 commented Jun 3, 2020

What about something like this?

const getAheadByCount = cache.function(async (latestTag: string, defaultBranch: string): Promise<number> => {
	let aheadCount = await fetchDom(`/${getRepoURL()}/releases/tag/${latestTag}`, '.release-header relative-time + a[href*="/compare/"]');
	if (!aheadCount) {
		aheadCount = await fetchDom(`/${getRepoURL()}/compare/${latestTag}...${defaultBranch}`, '#js-repo-pjax-container div.overall-summary > ul span');
	}

	// This text is "4 commits to master since this tag or 4 Commits"
	return looseParseInt(aheadCount!.textContent!);
}, {
	maxAge: 1 / 24, // One hour
	staleWhileRevalidate: 2,
	cacheKey: ([latestTag]) => `tag-ahead-by:${getRepoURL()}/${latestTag}`
});

@fregante
Copy link
Member Author

fregante commented Jun 3, 2020

Can't. Compare pages can be pretty heavy and slow as well though, just like the API.

This page takes a few seconds to load: 1.9.1...master

For the time being it would be good to just avoid that empty tooltip, for example by adding the tooltipped classes together with aria-label attribute, not before.

@fregante
Copy link
Member Author

fregante commented Jun 3, 2020

For the time being it would be good to just avoid that empty tooltip, for example by adding the tooltipped classes together with aria-label attribute, not before.

Done: e422c8c

@yakov116
Copy link
Member

yakov116 commented Jun 3, 2020

Could we make it a silent error?

@fregante
Copy link
Member Author

fregante commented Jun 6, 2020

I looked again into GraphQL but, while it has a before query, it appears to be completely ignored 🤷‍♂️

{
  repository(owner: "sindresorhus", name: "refined-github") {
    ref(qualifiedName: "master") {
      target {
        ... on Commit {
          id
          history(first: 100, before: "e422c8c7fe3f531816d0e0a7867c09b9acc7b266 1") {
            totalCount /* Also useless: it always includes ALL the commits */
            edges {
              node {
                message
              }
            }
          }
        }
      }
    }
  }
}

So instead I'm thinking we could:

  1. Get the latest tag’s sha

  2. Look for it in the latest 20 commits on defaultBranchRef

    {
      repository(owner: "sindresorhus", name: "refined-github") {
        defaultBranchRef {
          target {
            ... on Commit {
              history(first: 20) {
                edges {
                  node {
                    oid
    }}}}}}}}
  3. If found, use its position; if not, just use a number-less <sup>+</sup> in the latest-tag-button

After all, the point of this information is to know whether we're on the latest commit. Anything above +20 (or missing/incorrect information) isn't particularly useful.

@yakov116
Copy link
Member

yakov116 commented Jun 7, 2020

TADA It was a nice try, but commits can be out of order.

const getAheadByCount = cache.function(async (latestTag: string, defaultBranch: string): Promise<number | undefined> => {
	const tagPage = await fetchDom(
		`/${getRepoURL()}/releases/tag/${latestTag}`,
		'.release-header relative-time + a[href*="/compare/"], .release-header relative-time'
	);
	// This text is "4 commits to master since this tag"
	return tagPage instanceof HTMLAnchorElement ?
		Number(tagPage.textContent!.replace(/\D/g, '')) :
		getCommitAheadCountFromApi(tagPage!.attributes.datetime.value, defaultBranch);
}, {
	maxAge: 1 / 24, // One hour
	staleWhileRevalidate: 2,
	cacheKey: ([latestTag]) => `tag-ahead-by:${getRepoURL()}/${latestTag}`
});

const getCommitAheadCountFromApi = cache.function(async (date: string, defaultBranch: string): Promise<number> => {
	const {repository} = await api.v4(`
		repository(${getRepoGQL()}) {
			ref(qualifiedName: "${defaultBranch}") {
				target {
					... on Commit {
						history(first: 1, since: "${date}") {
							totalCount
						}
					}
				}
			}
		}
	`);

	return repository.ref.target.history.totalCount;
}, {
	maxAge: 1 / 24, // One hour
	staleWhileRevalidate: 2,
	cacheKey: ([latestTag]) => `tag-ahead-by-api:${getRepoURL()}/${latestTag}`
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants