fix: getData function should only change when getOptions changes - #166
Merged
Conversation
Updates the `useVisitorData` hook to only return a new `getData` function when the `getOptions` parameter is semantically different. Previously, the `getData` function would be recreated if the `getOptions` parameter was a different object instance but otherwise equivalent to the last invocation. The fix stores the `getOptions` parameter as a property of the state to ensure that the value used as a dependency in the `getCallback` call only changes when the semantics of the options for the `get` call change. The state is updated using a pattern to update the state of a component during a render by guarding that state update with a conditional. More information about this pattern can be found in the React docs: https://react.dev/reference/react/useState#storing-information-from-previous-renders The `usePrevious` utility was no longer used so it was removed. A new unit test is also added that validates the expected behavior of the `getData` function returned by `useVisitorData`. Fixes #132 Fixes INTER-1277
mcnulty-fp
requested review from
JuroUhlar,
TheUnderScorer and
ilfa
as code owners
June 3, 2025 13:02
Contributor
Coverage report
Test suite run success20 tests passing in 5 suites. Report generated by 🧪jest coverage report action from 65d4946 Show full coverage report
|
mcnulty-fp
commented
Jun 3, 2025
JuroUhlar
reviewed
Jun 4, 2025
JuroUhlar
previously approved these changes
Jun 4, 2025
JuroUhlar
left a comment
Contributor
There was a problem hiding this comment.
Nice PR! Appreciate the detailed explanatory comments in the test
Posted one nit, feel free to disregard and merge
TheUnderScorer
previously approved these changes
Jun 4, 2025
TheUnderScorer
previously approved these changes
Jun 4, 2025
JuroUhlar
reviewed
Jun 4, 2025
JuroUhlar
previously approved these changes
Jun 4, 2025
Co-authored-by: Juraj Uhlar <juro.uhlar@gmail.com>
Contributor
This PR will create a minor release 🚀2.7.0 (2025-06-04)Features
Bug FixesPerformance Improvements
Documentation
Build System
|
JuroUhlar
approved these changes
Jun 4, 2025
necipallef
approved these changes
Jun 16, 2025
Contributor
|
I like the solution and tests that cover it. Great job and great first PR! |
|
🎉 This PR is included in version 2.7.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
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.
Updates the
useVisitorDatahook to only return a newgetDatafunction when the
getOptionsparameter is semantically different.Previously, the
getDatafunction would be recreated if thegetOptionsparameter was a different object instance but otherwiseequivalent to the last invocation.
The fix stores the
getOptionsparameter as a property of the state toensure that the value used as a dependency in the
getCallbackcallonly changes when the semantics of the options for the
getcallchange. The state is updated using a pattern to update the state of
a component during a render by guarding that state update with a
conditional. More information about this pattern can be found in the
React docs:
https://react.dev/reference/react/useState#storing-information-from-previous-renders
The
usePreviousutility was no longer used so it was removed.A new unit test is also added that validates the expected behavior of
the
getDatafunction returned byuseVisitorData.Fixes #132
Fixes INTER-1277