Enhancements for useApiFetchCallback.#3521
Open
mikaeltomovic wants to merge 2 commits into
Open
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #3521 +/- ##
=========================================
- Coverage 64.1% 64.1% -0.0%
=========================================
Files 395 395
Lines 6773 6781 +8
Branches 1720 1723 +3
=========================================
+ Hits 4342 4344 +2
- Misses 2170 2173 +3
- Partials 261 264 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Changes proposed in this Pull Request:
Closes GOOWOO-8
Improves the
useApiFetchCallbackhook with two changes:Stabilize and memoize returned functions:
mergedStateis now stabilized viauseIsEqualRefValue+useMemo, and reset is wrapped inuseCallback. This makes bothenhancedApiFetchand reset maintain stable references across renders when their inputs haven't changed, preventing unnecessary re-renders for consumer components and avoiding spurious dependency changes in downstream hooks.Restrict
overwriteOptionstodataandqueryonly: The callback returned byuseApiFetchCallbackis stateful and bound to a specific API endpoint. Allowing arbitrary option overrides at call time (e.g.path,method) creates a misuse vector where the same hook instance could be used to call unrelated endpoints. LimitingoverwriteOptionstodata(request body) andquery(URL parameters) covers the only legitimate dynamic-at-call-time use cases. As part of this change, the five callsites that were passing{ parse: false }as an overwrite have been updated to declareparse: falseat hook instantiation instead.Screenshots:
N/A. Internal hook refactor with no UI changes.
Detailed test instructions:
switch-url-card), and URL reclaiming (reclaim-url-card) continue to work correctly, including the 403 reclaim flow.useUpsertAdsAccount,useCreateMCAccount,useConnectMCAccount) works without regressions.resetfunction correctly clears the error, then re-triggering the action works as expected.Additional details:
A third enhancement was considered, accepting an optional
initialDataargument to pre-populate thedatafield of the hook's state. This was not implemented because this capability already exists via the existinginitialStatesecond parameter (e.g.useApiFetchCallback( options, { data: someInitialValue } )). Adding a dedicatedinitialDataparameter would duplicate existing API surface without adding new capability, making the hook signature harder to reason about for no gain.Changelog entry