Introduce vendor namespace support#5
Conversation
|
Warning Review limit reached
Next review available in: 4 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a configurable vendor namespace to ChangesVendor namespace configuration
Estimated code review effort: 3 (Moderate) | ~20 minutes Sequence Diagram(s)sequenceDiagram
participant FlutterApp
participant ThunderIDConfig
participant NativeMethodHandler
participant EncryptedStorageAdapter
FlutterApp->>ThunderIDConfig: set vendor
ThunderIDConfig->>NativeMethodHandler: serialize vendor
NativeMethodHandler->>EncryptedStorageAdapter: derive vendor-scoped prefsName
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
565d271 to
28de7be
Compare
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.coderabbit.yaml (1)
10-24: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winConsider adding path_instructions for native file patterns.
The vendor naming rules in
AGENTS.mdare written generally, and the Android handler already derives storage names fromconfig.vendor. However, the automated review instructions only coverlib/**/*.dart. Adding similarpath_instructionsforandroid/**/*.ktandios/Classes/**/*.swiftwould ensure consistent automated detection of hardcoded brand literals in native code. This can be deferred if native SDK changes are still pending.As per coding guidelines, vendor naming rules apply to
android/**/*.ktandios/Classes/**/*.swiftas well.♻️ Suggested addition
- path: "lib/**/*.dart" instructions: | Flag any hardcoded occurrence of the literal `thunderid`/`ThunderID`/`THUNDERID` (any casing) used to build a runtime key or name — platform-channel storage keys, log tags, and similar. Do not flag entry point file/package names (e.g. `thunderid_flutter.dart`, `thunderid_client.dart`, `thunderid_provider.dart`) or types whose purpose IS to represent the SDK itself (e.g. `ThunderIDClient`, `ThunderIDProvider`). That's a fixed identity, not a per-tenant value. For everything else, ask: **"Should this read from `ThunderIDConfig.vendor` instead of hardcoding the vendor name?"** The best fix is avoiding the vendor name entirely when the brand prefix isn't load-bearing. When a brand-scoped namespace is genuinely required, it should resolve through `config.vendor` (which already defaults to `'thunderid'`) rather than a literal string. If the same default-resolution logic starts appearing in more than one place, ask for it to be extracted into a shared helper instead of repeated inline. + + - path: "android/**/*.kt" + instructions: | + Flag any hardcoded occurrence of the literal `thunderid`/`ThunderID`/`THUNDERID` (any casing) used to + build a runtime key or name — EncryptedSharedPreferences filenames, log tags, and similar. + + Do not flag fixed SDK identity types or entry-point class names. When a brand-scoped namespace is + genuinely required, it should resolve from the vendor value received via the MethodChannel rather + than a literal string. + + - path: "ios/Classes/**/*.swift" + instructions: | + Flag any hardcoded occurrence of the literal `thunderid`/`ThunderID`/`THUNDERID` (any casing) used to + build a runtime key or name — Keychain service identifiers, log tags, and similar. + + Do not flag fixed SDK identity types or entry-point class names. When a brand-scoped namespace is + genuinely required, it should resolve from the vendor value received via the MethodChannel rather + than a literal string.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.coderabbit.yaml around lines 10 - 24, Extend the vendor-naming review instructions in the configuration to cover native SDK files under android/**/*.kt and ios/Classes/**/*.swift, applying the same hardcoded ThunderID detection and config.vendor guidance already defined for lib/**/*.dart. Preserve the existing Dart path instructions and exclusions.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.coderabbit.yaml:
- Around line 10-24: Extend the vendor-naming review instructions in the
configuration to cover native SDK files under android/**/*.kt and
ios/Classes/**/*.swift, applying the same hardcoded ThunderID detection and
config.vendor guidance already defined for lib/**/*.dart. Preserve the existing
Dart path instructions and exclusions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: c917f155-5f29-4981-bd5d-b5accc844f1b
📒 Files selected for processing (5)
.coderabbit.yamlAGENTS.mdandroid/src/main/kotlin/dev/thunderid/flutter/ThunderIDMethodHandler.ktios/Classes/ThunderIDMethodHandler.swiftlib/src/models/thunderid_config.dart
…version CI was failing quickstart apk builds because AGP 8.2.2 is below Flutter's current minimum supported AGP version of 8.6.0.
…dency resolution Composite builds don't inherit repositories from the including build; without a project-level repositories block, Gradle can't resolve kotlin-stdlib, flutter_embedding_debug, or kotlinx-coroutines-android.
io.flutter:flutter_embedding_debug is published only to Flutter's own storage.googleapis.com/download.flutter.io maven repo, which is normally auto-added for registered Flutter plugin projects but not for the android module here since it's wired in via a manual includeBuild substitution.
…omposite build
The quickstart's includeBuild("../../../android") substituted
dev.thunderid:android with project(":") of its own composite build,
which resolves to this same android/ module and created a circular
dependency (:android depending on itself). dev.thunderid:android was
also never actually published to any repository.
Point the dependency at the real released artifact via JitPack
(com.github.thunder-id:android-sdks:v0.1.0) instead, and drop the
broken includeBuild/substitution. Also add the missing
android.useAndroidX=true gradle.properties for the android module,
since composite/included builds don't inherit properties from the
including project.
Purpose
ThunderIDis hardcoded in some SDK components, making it hard for adopters to white-label the SDK. Mirrors the approach landed in thunder-id/javascript-sdks#24.Approach
vendoroption toThunderIDConfig(defaults toThunderIDConfig.defaultVendor,'thunderid'), included intoMap()so it's sent over the MethodChannel.vendorthrough both native handlers (ios/Classes/ThunderIDMethodHandler.swift,android/.../ThunderIDMethodHandler.kt) into the nativeThunderIDConfig(vendor:)constructor.EncryptedSharedPreferencesfile name is now derived from the resolvedconfig.vendorinstead of a hardcoded string.AGENTS.mdso future changes keep runtime keys/names vendor-aware.Note
The native handler wiring targets the
vendorfield being added to the native SDKs in thunder-id/ios-sdks#7 and thunder-id/android-sdks#6. Until those land in published releases and this plugin's native dependencies (dev.thunderid:android,ThunderIDpod) are bumped, the native build may not pick upvendorsupport — the Dart-side API and MethodChannel plumbing are unaffected either way.Related Issues
Related PRs
Checklist
flutter analyzepasses with 0 issues.flutter testpasses (20/20).Summary by CodeRabbit
New Features
ThunderIDConfig.Documentation