Add a first-class expoRouterIntegration() with auto-registration (#6156)#6189
Add a first-class expoRouterIntegration() with auto-registration (#6156)#6189alwx wants to merge 4 commits into
Conversation
Semver Impact of This PR⚪ None (no version bump detected) 📋 Changelog PreviewThis is how your changes will appear in the changelog.
Plus 13 more 🤖 This preview updates automatically when you update the PR. |
|
|
@cursor review |
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 25475dc. Configure here.
antonis
left a comment
There was a problem hiding this comment.
Other than the lint issues that should be fixable with yarn fix and the agent comments LGTM
| if (hasTracingEnabled && options.enableAutoPerformanceTracing) { | ||
| integrations.push(expoRouterIntegration()); | ||
| } |
There was a problem hiding this comment.
Bug: User-provided options for expoRouterIntegration are ignored because a default instance is auto-registered first, preventing the user's configuration from being applied.
Severity: MEDIUM
Suggested Fix
Modify the integration setup logic to ensure that if a user-configured expoRouterIntegration exists, the default one is not added. Alternatively, merge the options from the user-provided instance into the existing one if it's already been created.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent. Verify if this is a real issue. If it is, propose a fix; if not, explain why it's
not valid.
Location: packages/core/src/js/integrations/default.ts#L142-L144
Potential issue: When tracing is enabled, `expoRouterIntegration()` is automatically
registered without options. If a user also manually provides `expoRouterIntegration`
with options (e.g., `enableTimeToInitialDisplay: true`), the final integration array
contains two instances. The first, default instance creates a
`reactNavigationIntegration`. The second, user-configured instance finds this existing
integration and reuses it, but fails to apply the user-provided options. As a result,
any options specified by the user for `expoRouterIntegration` are silently ignored.
Also affects:
samples/expo/app/_layout.tsx
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
I think the above is not an issue
| // Warning: (ae-forgotten-export) The symbol "ExpoRouterIntegrationOptions" needs to be exported by the entry point index.d.ts | ||
| // | ||
| // @public | ||
| export const expoRouterIntegration: (options?: Partial<ExpoRouterIntegrationOptions>) => Integration; |
There was a problem hiding this comment.
This would need to change after the latest fixes
| export const expoRouterIntegration: (options?: Partial<ExpoRouterIntegrationOptions>) => Integration; | |
| export const expoRouterIntegration: (options?: ExpoRouterIntegrationOptions) => Integration; |
antonis
left a comment
There was a problem hiding this comment.
Other than the api change issue LGMT!

📢 Type of change
📜 Description
Fixes #6156
📝 Checklist
sendDefaultPIIis enabled🔮 Next steps
Docs need to be updated as well