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

Rewrite RTKQ internal subscription lookups and subscription syncing #3824

Merged

Conversation

markerikson
Copy link
Collaborator

@markerikson markerikson commented Oct 26, 2023

  • Changed subscriptionUpdated from a microtask to a 500ms throttle
  • Exposed the RTKQ internal middleware state to be returned via an
    internal action, so that hooks can read that state directly without
    needing to call dispatch() on every render.

    Updated the batchActions middleware to return a SubscriptionSelectors object with {getSubscriptions, getSubscriptionCount, isRequestSubscribed}, and used that in the hook (and tests)
  • Reworked tests to completely ignore subscriptionsUpdated in any
    action sequence checks due to timing changes and irrelevance
  • Fixed case where invalidateTags was still reading from store state

- Changed `subscriptionUpdated` from a microtask to a 500ms throttle
- Exposed the RTKQ internal middleware state to be returned via an
  internal action, so that hooks can read that state directly without
  needing to call `dispatch()` on every render.
- Reworked tests to completely ignore `subscriptionsUpdated` in any
  action sequence checks due to timing changes and irrelevance
- Fixed case where `invalidateTags` was still reading from store state
@codesandbox
Copy link

codesandbox bot commented Oct 26, 2023

Review or Edit in CodeSandbox

Open the branch in Web EditorVS CodeInsiders

Open Preview

@markerikson markerikson changed the title keep subscription on data while query is running Rewrite RTKQ internal subscription lookups and subscription syncing Oct 26, 2023
@codesandbox-ci
Copy link

codesandbox-ci bot commented Oct 26, 2023

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit e0c3869:

Sandbox Source
Vanilla Configuration
Vanilla Typescript Configuration
rsk-github-issues-example Configuration
@examples-query-react/basic Configuration
@examples-query-react/advanced Configuration
@examples-action-listener/counter Configuration
rtk-esm-cra Configuration

@@ -681,6 +682,27 @@ export function buildHooks<Definitions extends EndpointDefinitions>({
Definitions
>
const dispatch = useDispatch<ThunkDispatch<any, any, UnknownAction>>()
const internalStateRef = useRef<InternalMiddlewareState | null>(null)
Copy link
Member

@phryneas phryneas Oct 26, 2023

Choose a reason for hiding this comment

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

This logic in the hooks file hurts a bit. Maybe the middleware could return a hasSubscription function instead of the internal state itself?

Comment on lines +76 to +77
const subscriptionSubState =
internalState.currentSubscriptions[queryCacheKey] ?? {}
Copy link
Member

Choose a reason for hiding this comment

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

Thank you for catching this one!

@markerikson markerikson force-pushed the feature/3709-alter-subscription-updates branch from 2fc1ee6 to e0c3869 Compare October 27, 2023 01:54
@markerikson markerikson merged commit 6da44eb into pr/running_subscription Oct 27, 2023
37 checks passed
@markerikson markerikson deleted the feature/3709-alter-subscription-updates branch October 27, 2023 02:11
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.

None yet

2 participants