Conversation
🦋 Changeset detectedLatest commit: 3d329a4 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
…egration Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
|
|
||
| return () => { | ||
| isSubscribed = false; | ||
| lastFetchRef.current = null; // Reset fetch tracking on unmount or dependency change |
There was a problem hiding this comment.
Instead of setting lastFetchRef.current to null, can we let the dependency array trigger new fetches when needed. That way only when payload/instance changes it triggers a new fetch.
There was a problem hiding this comment.
Good question. I think this fix was related to StrictMode double mounting practice.
There was a problem hiding this comment.
Hi @nityasp This is to pass our StrictMode test in react, Consider this flow:
Step: Mount 1
Without = null: fetch, lastFetchRef = {A, X}
With = null: fetch, lastFetchRef = {A, X}
────────────────────────────────────────
Step: Unmount
Without = null: isSubscribed=false, lastFetchRef = {A, X}
With = null: isSubscribed=false, lastFetchRef=null
────────────────────────────────────────
Step: Mount 2
Without = null: hasFetchedThisConfig=true, will skip fetch
With = null: hasFetchedThisConfig=false, will do a new fetch
────────────────────────────────────────
Step: Result
without = null: ❌ The first fetch's dispatch won't execute (isSubscribed=false), no data of eligibleMethods
with = null: ✅ Second fetch can dispatch and there's result stored in context
I hope this illustration can clarify your confusion. Lmk if you have any other questions. Thank you!
There was a problem hiding this comment.
Thanks for explaining it in detail. I understood it.
There was a problem hiding this comment.
Just a minor thing. I think we don't need eligibilePaymentMethods in the dependency array. This is the output of the effect and this causes the effect to re-run everytime there is a successful fetch.
There was a problem hiding this comment.
Good catch! Adopted the change. @EvanReinstein What do you think? I believe eligibilePaymentMethods would cause useEffect to run again(tho prevented by if check).
…ypal/paypal-js into fix/use-eligible-methods-hook
| // isLoading should be true if: | ||
| // 1. We're actively fetching, OR | ||
| // 2. We don't have eligibility data yet and no error occurred | ||
| // This prevents a flash where isLoading=false before the effect runs | ||
| const isLoading = | ||
| isFetching || (!eligiblePaymentMethods && !eligibilityError); |
EvanReinstein
left a comment
EvanReinstein
left a comment
There was a problem hiding this comment.
Great work on this one!
3 participants
No description provided.