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

Some features show information from the wrong/previous repository visited #6988

Closed
134130 opened this issue Oct 17, 2023 · 12 comments · Fixed by #7061
Closed

Some features show information from the wrong/previous repository visited #6988

134130 opened this issue Oct 17, 2023 · 12 comments · Fixed by #7061
Assignees
Labels
ajax Temporary label to collect Ajax issues bug help wanted Please! ♥︎ Particularly useful features that everyone would love!

Comments

@134130
Copy link
Contributor

134130 commented Oct 17, 2023

Description

  • repo-header-info show invalid repository's stargazer-count.
  • image

How to replicate the issue + URL

  1. Clear cache of refined-github
  2. Go to https://github.com/refined-github/refined-github
  3. Go to it's organization with Left top's breadcrumb
    image
  4. Go to other repository
  5. Check your repo-header-info stargazer-count

Extension version

23.10.5

Browser(s) used

Chrome

@134130 134130 added the bug label Oct 17, 2023
@134130
Copy link
Contributor Author

134130 commented Oct 17, 2023

The reason is CachedFunction's cacheKey is initialized with refined-github/refined-github, and visiting other repository with out script reloading (soft-rendering), CachedFunction still has cacheKey to refined-github/refined-github

https://github.com/refined-github/refined-github/blob/main/source/features/repo-header-info.tsx#L20-L28

There can be same cache issues which initializes CachedFunction like this case.

@fregante
Copy link
Member

That cacheKey is dynamic, it depends on location.href

Maybe the issue is that it's fired at the wrong time due to:

@134130
Copy link
Contributor Author

134130 commented Oct 17, 2023

That cacheKey is dynamic, it depends on location.href

Maybe the issue is that it's fired at the wrong time due to:

Yes. I've checked cacheKey is correctlly updated on feature init callback.
But, the cacheKey is passed to CachedFunction's constuctor as value, cannot apply location.href dynamically, when soft-rendering cases.

Fix : My bad. CacheKey is not related. Maybe you are right. We need to figure out why gql is querying with previous repository. I'll check it later.

@fregante fregante changed the title repo-header-info shows invalid repo's stargazer-count Some features show information from the wrong/previous repository visited Nov 8, 2023
@fregante fregante added ajax Temporary label to collect Ajax issues help wanted labels Nov 8, 2023
@fregante
Copy link
Member

fregante commented Nov 8, 2023

I have a repro for another feature, default-branch-button:

  1. Clear cache
  2. Open this in a new tab https://github.com/sindresorhus/p-retry/releases/tag/v6.1.0
  3. Click "separate package"
  4. Open a file or folder
Screen.Recording.mov

@fregante fregante added the Please! ♥︎ Particularly useful features that everyone would love! label Nov 8, 2023
@fregante
Copy link
Member

fregante commented Nov 8, 2023

Oof, it doesn't simply mess up the current view, but it also messes up the cache. I thought this was just a few issue, but for some reason it takes the previous value and stores it on the current URL.

My guess is that getRepo is called at different times, or that webext-storage-cache is memoizing the function call incorrectly.

@jplatte

This comment was marked as resolved.

@fregante

This comment was marked as resolved.

@jplatte

This comment was marked as resolved.

@fregante

This comment was marked as resolved.

@jplatte

This comment was marked as resolved.

@134130
Copy link
Contributor Author

134130 commented Nov 9, 2023

Not sure, but as you said, It appeares frequently and commonly on other features also.

@fregante fregante pinned this issue Nov 9, 2023
@fregante
Copy link
Member

fregante commented Nov 15, 2023

The issue is:

export const v4uncached = async (
query: string,
options: GHGraphQLApiOptions = v4defaults,
): Promise<AnyObject> => {
const personalToken = await expectToken();
// TODO: Remove automatic usage of globals via `getRepo()`
// https://github.com/refined-github/refined-github/issues/5821
const currentRepoIfAny = getRepo(); // Don't destructure, it's `undefined` outside repos
query = query.replace('repository() {', () => 'repository(owner: $owner, name: $name) {');

I thought I had fixed this in

export const v4 = mem(v4uncached, {
cacheKey([query, options]) {
// `repository()` uses global state and must be handled explicitly
// https://github.com/refined-github/refined-github/issues/5821
// https://github.com/sindresorhus/eslint-plugin-unicorn/issues/1864
const key = [query, options];
if (query.includes('repository() {')) {
key.push(getRepo()?.nameWithOwner);
}
return JSON.stringify(key);
},
});

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ajax Temporary label to collect Ajax issues bug help wanted Please! ♥︎ Particularly useful features that everyone would love!
4 participants
@jplatte @fregante @134130 and others