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

cache global store get requests #34

Merged
merged 1 commit into from
Jan 26, 2024
Merged

Conversation

tanbowensg
Copy link
Member

@tanbowensg tanbowensg commented Jan 26, 2024

在应用启动时,有可能短时间内多次调用global-store的get方法。此时,第一个请求都尚未返回,所以没有缓存,因此就会连续发送多个请求。
截屏2024-01-26 14 06 32

这个pr的作用就是缓存get产生的promise。当没有data的缓存,并且请求参数和之前完全相同的话,就返回缓存的promise,而不重复发送请求。

@tanbowensg tanbowensg force-pushed the bowen/get-cache-request branch from 106b05f to c52b927 Compare January 26, 2024 06:15
const cacheRequestIndex = this.requestsCache.findIndex(f => {
return f.promise === promise;
});
this.requestsCache.splice(cacheRequestIndex, 1);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

应该 cacheRequestIndex 不为 -1 再执行,onResponse 应该会被多次调用的,防止意外移除其他的。或者放到下面 then 那里处理也可以?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok

.then(stop => {
this.stopWatchHandlers.set(resource, stop);
})
.catch(e => reject(e));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

catch 这里应该也要移除 requestsCache

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

最后加到finally里面去了

@tanbowensg tanbowensg force-pushed the bowen/get-cache-request branch from c52b927 to 280092b Compare January 26, 2024 07:33
@tanbowensg tanbowensg merged commit 7ff17ce into main Jan 26, 2024
1 check passed
@tanbowensg tanbowensg deleted the bowen/get-cache-request branch January 26, 2024 09:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants