-
-
Notifications
You must be signed in to change notification settings - Fork 0
chore: sync upstream PR #8256 - feat(core): export native bridge utility functions #37
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
base: plus
Are you sure you want to change the base?
Conversation
- Create android/README.md with installation, usage, and configuration details - Create ios/README.md with installation, usage, and configuration details - Include version compatibility tables - Add links to official documentation and resources This ensures all core packages have proper documentation on npm and GitHub.
- Export isRelativeOrProxyUrl and createProxyUrl on Capacitor global object - Update CapacitorInstance interface to include new methods - Remove TODO comments in native-bridge.ts This allows plugins and other parts of the system to use these utility functions for URL handling.
WalkthroughDocumentation for Capacitor Android and iOS runtimes was added, covering installation, usage, configuration, and compatibility. Two internal helper functions— Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
41a9b2e to
bc202b1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (2)
core/src/definitions-internal.ts (1)
93-101: New helper methods onCapacitorInstancetighten the type and may affect custom implementorsAdding
isRelativeOrProxyUrlandcreateProxyUrlas required members is consistent with howinitBridgewires them on the runtime instance, and it cleanly exposes the native-bridge helpers.One nuance: because
CapacitorInstanceis now stricter, any custom platform/runtime code that structurally implements this interface may start failing type-checks until these methods are added. If you intend this to be purely additive for consumers, you might consider making these properties optional in the type (while still assigning them at runtime), or at least document the expectation for custom implementations.core/native-bridge.ts (1)
125-138: Helper exposure matches existing bridge logic; be aware ofcreateProxyUrlusage constraintsThe new
isRelativeOrProxyUrlandcreateProxyUrlhelpers, and their exposure viacap.isRelativeOrProxyUrl/cap.createProxyUrl, align with the existing native-bridge.js implementations and with the internal uses in the fetch/XHR patches. This is a clean way to let plugins and app code reuse the same URL/proxy logic as the core bridge.One subtle point:
createProxyUrlbuilds its base withnew URL(win.Capacitor?.getServerUrl() ?? ''). In standard native contextsgetServerUrl()should be a valid origin, but if a consumer callscap.createProxyUrl()in an environment whereWEBVIEW_SERVER_URLis unset or non-URL (e.g., non-native/web usage), this can throw at runtime. That was already implicitly true for the internal use, but exposing it as public API makes external misuse more likely.It may be worth:
- Documenting that
createProxyUrlis intended for native/webview contexts wheregetServerUrl()is a valid URL, and/or- Optionally short-circuiting to return the original URL when
getServerUrl()is falsy/invalid, if you want a more defensive public helper.Also applies to: 238-240
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
android/README.md(1 hunks)core/native-bridge.ts(1 hunks)core/src/definitions-internal.ts(1 hunks)ios/README.md(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
core/native-bridge.ts (2)
ios/Capacitor/Capacitor/assets/native-bridge.js (2)
isRelativeOrProxyUrl(144-144)createProxyUrl(146-154)android/capacitor/src/main/assets/native-bridge.js (2)
isRelativeOrProxyUrl(144-144)createProxyUrl(146-154)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: test-android
- GitHub Check: test-core
🔇 Additional comments (2)
ios/README.md (1)
1-75: iOS runtime README looks consistent and safe to mergeThe installation, usage, configuration, and version-compat sections are clear and align with typical Capacitor iOS setup. No security or behavioral concerns from this doc-only addition.
android/README.md (1)
1-73: Android runtime README mirrors existing patterns with no behavioral impactInstall/add/run instructions, manifest/permissions guidance, version matrix, and resources look appropriate and match typical Capacitor Android usage. No security or breaking-change concerns.
Upstream PR Sync
This PR syncs changes from an external contributor's PR on the official Capacitor repository.
Original PR
Automation
Synced from upstream by Capacitor+ Bot
Summary by CodeRabbit
New Features
isRelativeOrProxyUrl()for URL validation andcreateProxyUrl()for URL proxy handling.Documentation
✏️ Tip: You can customize this high-level summary in your review settings.