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

feat: custom built fp and logger in debug mode #113

Merged
merged 3 commits into from
Jul 18, 2024

Conversation

aversini
Copy link
Collaborator

@aversini aversini commented Jul 18, 2024

PR Type

Enhancement, Dependencies


Description

  • Added debug prop to AuthProvider and InternalContext for enabling debug mode.
  • Implemented a custom logger function in AuthProvider to log messages when debug is enabled.
  • Replaced getFingerprint with getFingerprintHash for fingerprint generation.
  • Simplified login logic and removed redundant console logging in main.tsx.
  • Updated dependencies by removing @thumbmarkjs/thumbmarkjs and adding @versini/ui-fingerprint.

Changes walkthrough 📝

Relevant files
Enhancement
index.tsx
Add debug mode to AuthProvider in index.tsx                           

examples/code-flow/src/index.tsx

  • Added debug prop to AuthProvider component.
+1/-0     
main.tsx
Simplify login logic and remove redundant logging               

examples/code-flow/src/main.tsx

  • Removed console logging for isAuthenticated and isLoading.
  • Simplified login and error handling logic.
  • +2/-20   
    types.d.ts
    Add debug property to AuthProvider types                                 

    packages/auth-provider/src/common/types.d.ts

  • Added debug property to AuthProviderProps and AuthState types.
  • +2/-0     
    utilities.ts
    Replace getFingerprint with getFingerprintHash                     

    packages/auth-provider/src/common/utilities.ts

    • Replaced getFingerprint with getFingerprintHash.
    +2/-8     
    AuthProvider.tsx
    Add debug mode and custom logger to AuthProvider                 

    packages/auth-provider/src/components/AuthProvider/AuthProvider.tsx

  • Added debug prop to AuthProvider.
  • Implemented custom logger function.
  • Used fingerprintRef for fingerprint value.
  • Added debug logging throughout the component.
  • +52/-7   
    InternalContext.ts
    Add debug property to InternalContext                                       

    packages/auth-provider/src/components/AuthProvider/InternalContext.ts

    • Added debug property to InternalContext.
    +1/-0     
    Dependencies
    package.json
    Update dependencies in package.json                                           

    packages/auth-provider/package.json

  • Removed @thumbmarkjs/thumbmarkjs dependency.
  • Added @versini/ui-fingerprint dependency.
  • +2/-4     
    pnpm-lock.yaml
    Update lock file for new dependencies                                       

    pnpm-lock.yaml

  • Removed @thumbmarkjs/thumbmarkjs package.
  • Added @versini/ui-fingerprint package.
  • +8/-8     

    💡 PR-Agent usage:
    Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

    @qodo-merge-pro qodo-merge-pro bot added enhancement New feature or request dependencies Pull requests that update a dependency file Review effort [1-5]: 4 labels Jul 18, 2024
    Copy link

    PR Reviewer Guide 🔍

    ⏱️ Estimated effort to review: 4 🔵🔵🔵🔵⚪
    🧪 No relevant tests
    🔒 No security concerns identified
    ⚡ Key issues to review

    Debug Logging
    The debug logging mechanism is implemented using console.info, which might not be suitable for production environments. Consider using a more sophisticated logging library that supports different levels of logging and can be easily turned off or redirected to other outputs in production.

    Error Handling
    The error handling in getAccessToken and other methods logs errors but does not rethrow them or handle them in a way that the calling context can react to these errors. This might lead to unhandled exceptions or errors that are swallowed silently.

    Async useEffect
    The useEffect hook uses an async function directly which is an anti-pattern because it can lead to memory leaks if the component unmounts before the async operation completes. Consider refactoring to use a flag that checks if the component is still mounted before setting state.

    Copy link

    qodo-merge-pro bot commented Jul 18, 2024

    PR Code Suggestions ✨

    CategorySuggestion                                                                                                                                    Score
    Best practice
    ✅ Add cleanup logic to useEffect to prevent potential memory leaks

    To prevent potential memory leaks or unnecessary executions, add cleanup logic in
    the useEffect for setting the fingerprintRef to clear the fingerprintRef.current
    when the component unmounts.

    packages/auth-provider/src/components/AuthProvider/AuthProvider.tsx [135-139]

     useEffect(() => {
    +  let isActive = true;
       (async () => {
    -    fingerprintRef.current = await getCustomFingerprint();
    +    const fingerprint = await getCustomFingerprint();
    +    if (isActive) {
    +      fingerprintRef.current = fingerprint;
    +    }
       })();
    +  return () => {
    +    isActive = false;
    +    fingerprintRef.current = "";
    +  };
     }, []);
     

    [Suggestion has been applied]

    Suggestion importance[1-10]: 8

    Why: Adding cleanup logic to the useEffect hook is important to prevent potential memory leaks, especially in a component that handles authentication. This improves the robustness of the code.

    8
    ✅ Enhance type safety in the logger function
    Suggestion Impact:The commit changed the type of the args parameter in the logger function from 'any[]' to 'unknown[]', improving type safety as suggested.

    code diff:

    -		(...args: any[]) => {
    +		(...args: unknown[]) => {

    Consider using a more specific type for the args parameter in the logger function to
    ensure type safety and better integration with TypeScript's type system.

    packages/auth-provider/src/components/AuthProvider/AuthProvider.tsx [75]

    -(...args: any[])
    +(...args: unknown[])
     
    Suggestion importance[1-10]: 6

    Why: Using 'unknown[]' instead of 'any[]' improves type safety, which is a good practice in TypeScript. However, the impact on the overall functionality is minor.

    6
    Enhancement
    Improve the structure of debug logging

    Replace the hardcoded debug logging strings with a more structured format, possibly
    using an object to include both a message and additional data. This will improve the
    readability and maintainability of the logs, especially when debugging complex
    issues.

    packages/auth-provider/src/components/AuthProvider/AuthProvider.tsx [77]

    -console.info(`==> [Auth ${Date.now()}]: `, ...args);
    +console.info({ message: `==> [Auth ${Date.now()}]: `, data: args });
     
    Suggestion importance[1-10]: 7

    Why: The suggestion improves the readability and maintainability of the logs by using a more structured format, which is beneficial for debugging complex issues. However, it is not a critical change.

    7
    Maintainability
    Consolidate repeated logout calls into a single function to improve maintainability

    Refactor the repeated await invalidateAndLogout(ACCESS_TOKEN_ERROR); calls into a
    single function call within the catch block to reduce redundancy and improve code
    maintainability.

    packages/auth-provider/src/components/AuthProvider/AuthProvider.tsx [305-317]

    -await invalidateAndLogout(ACCESS_TOKEN_ERROR);
    -await invalidateAndLogout(ACCESS_TOKEN_ERROR);
    +try {
    +  // existing logic
    +} catch (_error) {
    +  await handleLogoutOnError(ACCESS_TOKEN_ERROR);
    +}
     
    • Apply this suggestion
    Suggestion importance[1-10]: 7

    Why: Refactoring repeated calls into a single function improves code maintainability and reduces redundancy, which is beneficial for long-term code management. However, it does not address a critical issue.

    7

    Copy link

    Bundle Size

    Status File Size (Gzip) Limits
    index.js 17.53 KB (-3.04 KB -14.78%) 18 kb

    Overall bundle size: 17.53 KB (-3.04 KB -14.78%)
    Overall status: ✅

    @aversini aversini merged commit b1b131c into main Jul 18, 2024
    4 checks passed
    @aversini aversini deleted the feat-custom-built-fp-and-logger-in-debug-mode branch July 18, 2024 15:45
    @aversini aversini mentioned this pull request Jul 18, 2024
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    dependencies Pull requests that update a dependency file enhancement New feature or request Review effort [1-5]: 4
    Projects
    None yet
    Development

    Successfully merging this pull request may close these issues.

    1 participant