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 cascade of requests firing #569

Merged
merged 1 commit into from
Oct 23, 2023

Conversation

sergei-maertens
Copy link
Member

Closes #546

Hook dependencies operate on the identity, and the identity of the selectedProducts changes every time on each render, while the contents stay the same.

JSON.stringify is the recommended approach to mitigate using non-primivites in hook dependency arrays.

Note that the caching masked the issue partially - it caused the request cascade to stop, but the problem still showed with multi- product appointments.

@sergei-maertens sergei-maertens added the needs-backport Fix must be backported to stable release branch label Oct 19, 2023
Hook dependencies operate on the identity, and the identity of
the selectedProducts changes every time on each render, while the
contents stay the same.

JSON.stringify is the recommended approach to mitigate using non-primivites
in hook dependency arrays.

Note that the caching masked the issue partially - it caused the
request cascade to stop, but the problem still showed with multi-
product appointments.
@sergei-maertens sergei-maertens force-pushed the issue/excessive-appointment-products-requests branch from c10f5c5 to c8c9684 Compare October 19, 2023 07:46
@codecov
Copy link

codecov bot commented Oct 19, 2023

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (b3b2a21) 63.64% compared to head (c8c9684) 70.71%.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #569      +/-   ##
==========================================
+ Coverage   63.64%   70.71%   +7.06%     
==========================================
  Files         191      209      +18     
  Lines        4162     4319     +157     
  Branches     1127     1171      +44     
==========================================
+ Hits         2649     3054     +405     
+ Misses       1513     1230     -283     
- Partials        0       35      +35     
Files Coverage Δ
...rc/components/appointments/fields/ProductSelect.js 100.00% <ø> (ø)

... and 74 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@SilviaAmAm SilviaAmAm self-assigned this Oct 23, 2023
@sergei-maertens sergei-maertens merged commit af10edd into main Oct 23, 2023
12 checks passed
@sergei-maertens sergei-maertens deleted the issue/excessive-appointment-products-requests branch October 23, 2023 14:51
sergei-maertens added a commit that referenced this pull request Oct 23, 2023
Hook dependencies operate on the identity, and the identity of
the selectedProducts changes every time on each render, while the
contents stay the same.

JSON.stringify is the recommended approach to mitigate using non-primivites
in hook dependency arrays.

Note that the caching masked the issue partially - it caused the
request cascade to stop, but the problem still showed with multi-
product appointments.

Backport-of: #569
@sergei-maertens
Copy link
Member Author

Backport: 78b1b76

1.4.x and older are not affected, new appointments flow is since 1.5.x

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
needs-backport Fix must be backported to stable release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Appointments: multi-product makes duplicate requests
2 participants