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

fix(discover): Should reload when payload changes while loading #28653

Merged

Conversation

Zylphrex
Copy link
Member

If the eventView changes while the request is in-flight/loading, we should
cancel the in-flight request and fetch again. This is problematic in cases where
the request takes a little longer to load and the user changes the query while
it is still in-flight. If a reload is not triggered at this time, the results
are actually incorrect since it's showing the results from the previous (stale)
in-flight request.

If the eventView changes while the request is in-flight/loading, we should
cancel the in-flight request and fetch again. This is problematic in cases where
the request takes a little longer to load and the user changes the query while
it is still in-flight. If a reload is not triggered at this time, the results
are actually incorrect since it's showing the results from the previous (stale)
in-flight request.
@Zylphrex Zylphrex requested review from a team as code owners September 17, 2021 15:31
Comment on lines +202 to +203
// clear any inflight requests since they are now stale
api.clear();
Copy link
Member Author

Choose a reason for hiding this comment

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

Does this have the potential to cancel other requests unintentionally? 🤔

Copy link
Member

Choose a reason for hiding this comment

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

The cancellation only occurs for the Client instance of api. Not other Client instances.

Copy link
Member

Choose a reason for hiding this comment

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

basically it depends on how you pass in api, I think perhaps we should make these instances unique now with useAPI()

Copy link
Member

@k-fish k-fish left a comment

Choose a reason for hiding this comment

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

Given that you'd audited the api props, 👍

@Zylphrex Zylphrex merged commit 25e29fe into master Sep 23, 2021
@Zylphrex Zylphrex deleted the fix/should-reload-when-payload-changes-while-loading branch September 23, 2021 14:21
vuluongj20 pushed a commit that referenced this pull request Sep 30, 2021
If the eventView changes while the request is in-flight/loading, we should
cancel the in-flight request and fetch again. This is problematic in cases where
the request takes a little longer to load and the user changes the query while
it is still in-flight. If a reload is not triggered at this time, the results
are actually incorrect since it's showing the results from the previous (stale)
in-flight request.
@github-actions github-actions bot locked and limited conversation to collaborators Oct 9, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants