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 useHandleCallback compatibility with React.StrictMode #10486

Merged
merged 2 commits into from
Jan 31, 2025

Conversation

erwanMarmelab
Copy link
Contributor

@erwanMarmelab erwanMarmelab commented Jan 30, 2025

Problem

Many authentication providers do not accept calling their methods for validating authentication callbacks multiple times. We had the case with ra-auth-auth0 and ra-keycloak (maybe even ra-supabase).

Our solution until now was to implement the necessary logic in the authProvider implementation (storing ongoing promise in the authProvider closure). This is fine but it means custom implementations must do it themselves too and this is not trivial for many.

Solution

Make the useHandleCallback resilient to StrictMode. Implement the logic that won’t call authProvider.handleCallback if another call is already ongoing.

How To Test

Revert the fix in useHandleAuthCallback.ts and run the test "should only call handleCallback once when the component is rendered" in useHandleAuthCallback.spec.tsx.

Additional Checks

  • The PR targets master for a bugfix, or next for a feature
  • The PR includes unit tests (if not possible, describe why)
  • [ ] The PR includes one or several stories (if not possible, describe why)
  • [ ] The documentation is up to date

Screenshot

Test without the fix:

image

@erwanMarmelab erwanMarmelab added the RFR Ready For Review label Jan 30, 2025
Comment on lines 34 to 55
handleCallbackPromise = new Promise(async (resolve, reject) => {
if (authProvider) {
if (typeof authProvider.handleCallback === 'function') {
try {
const result =
await authProvider.handleCallback({
signal,
});
return resolve(result ?? null);
} catch (error) {
return reject({
redirectTo: false,
message: error.message,
});
}
}
return resolve();
}
return reject({
message: 'Failed to handle login callback.',
});
});
Copy link
Collaborator

Choose a reason for hiding this comment

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

This changes the previous behavior. Please revert to the original code. Only keep the handleCallbackPromise creation as you did

@djhi djhi added this to the 5.5.3 milestone Jan 31, 2025
@djhi djhi merged commit 6ef98e7 into master Jan 31, 2025
16 checks passed
@djhi djhi deleted the fix/useHandleCallback_compatible_StrictMode branch January 31, 2025 10:12
@djhi djhi changed the title Make useHandleCallback compatible with StrictMode Fix useHandleCallback compatibility with React.StrictMode Jan 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFR Ready For Review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants