Skip to content

Fix: unwrap nested data response from React Router#44

Merged
chaance merged 1 commit into
workos:mainfrom
DennisKraaijeveld:fix/nested-data-response
Dec 12, 2025
Merged

Fix: unwrap nested data response from React Router#44
chaance merged 1 commit into
workos:mainfrom
DennisKraaijeveld:fix/nested-data-response

Conversation

@DennisKraaijeveld
Copy link
Copy Markdown
Contributor

@DennisKraaijeveld DennisKraaijeveld commented Nov 16, 2025

This PR fixes an issue in authkitLoader where the loader response was wrapped twice in DataWithResponseInit, producing a nested structure that React Router 7 cannot interpret correctly during client-side navigations. The result was that loader data ended up one level too deep, making fields inaccessible and breaking downstream consumers.

authkitLoader internally wrapped values with data(...) even when the user-supplied loader already returned a data(...) response.The incorrect implementation produced:

DataWithResponseInit {
  data: {
    type: 'DataWithResponseInit',  ← invalid nested wrapper
    data: { /* actual payload */ }
  }
}

This may resolve issue #38, as the symptoms described there match the nested data structure, though this has not been explicitly validated.

@DennisKraaijeveld DennisKraaijeveld marked this pull request as ready for review November 16, 2025 00:31
@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps Bot commented Nov 16, 2025

Greptile Overview

Greptile Summary

This PR fixes a critical bug where authkitLoader double-wrapped loader responses in DataWithResponseInit, breaking client-side navigation in React Router 7.

Key Changes:

  • Added UnwrapData<T> utility type in src/interfaces.ts:6 to extract inner types from DataWithResponseInit
  • Updated handleAuthLoader in src/session.ts:481-494 to detect and unwrap nested data() responses before merging with auth data
  • Updated type signatures in src/session.ts:259,294 to use UnwrapData<Data> ensuring type safety for unwrapped responses
  • Improved header merging logic to preserve headers from user-returned data() responses

Impact:
The fix ensures that when user loaders return data({categories: []}), the library correctly extracts the inner data and headers before re-wrapping once with auth fields, preventing the nested structure that caused props.loaderData.data.categories instead of props.loaderData.categories.

Confidence Score: 4/5

  • This PR is safe to merge with minor concerns about test coverage
  • The fix correctly addresses the nested data structure issue with proper type safety via UnwrapData utility. Implementation handles all three return types (Response, DataWithResponseInit, plain object). However, there are no explicit test cases validating the fix for the specific scenario where user loaders return data() wrappers
  • Consider adding test coverage in src/session.spec.ts for the scenario where custom loaders return data() wrapped responses

Important Files Changed

File Analysis

Filename Score Overview
src/interfaces.ts 5/5 Added UnwrapData utility type to extract inner type from DataWithResponseInit wrapper
src/session.ts 4/5 Fixed nested DataWithResponseInit issue by unwrapping user-returned data() responses before re-wrapping with auth data

Sequence Diagram

sequenceDiagram
    participant User as User Loader
    participant AuthKit as authkitLoader
    participant HandleAuth as handleAuthLoader
    participant RR as React Router
    
    User->>AuthKit: Returns data({categories: []})
    Note over User: DataWithResponseInit wrapper
    
    AuthKit->>HandleAuth: Passes loader result
    
    alt Loader returns Response
        HandleAuth->>RR: Handles as Response
    else Loader returns data() wrapper (FIXED)
        HandleAuth->>HandleAuth: Check isDataWithResponseInit()
        Note over HandleAuth: Unwrap: loaderResult.data
        HandleAuth->>HandleAuth: Extract headers from loaderResult.init
        HandleAuth->>HandleAuth: Merge data with auth
        HandleAuth->>RR: Return data(mergedData, mergedHeaders)
        Note over RR: Single level wrapping ✓
    else Loader returns plain object
        HandleAuth->>HandleAuth: Use loaderResult directly
        HandleAuth->>HandleAuth: Merge data with auth
        HandleAuth->>RR: Return data(mergedData, headers)
        Note over RR: Single level wrapping ✓
    end
Loading

Copy link
Copy Markdown
Contributor

@greptile-apps greptile-apps Bot left a comment

Choose a reason for hiding this comment

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

2 files reviewed, no comments

Edit Code Review Agent Settings | Greptile

@DennisKraaijeveld
Copy link
Copy Markdown
Contributor Author

@nicknisi Not sure who to mention but could someone review and possibly merge this? :) Currently I use a local fork in my app as a temporary workaround but would be nice to see this fixed upstream as well.

@chaance chaance merged commit d2a3d36 into workos:main Dec 12, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants