feat: support transitive dependencies in BGP (RNC-CLI path) - #458
Open
KisaneNeko wants to merge 17 commits into
Open
feat: support transitive dependencies in BGP (RNC-CLI path)#458KisaneNeko wants to merge 17 commits into
KisaneNeko wants to merge 17 commits into
Conversation
…out of expo package, add test infra Move DependencyInfo and VersionMediatingDependencySet from expo.utils to shared package to make them available for transitive dependency handling in the BGP. Add JUnit 5 test infrastructure and initial regression test for VersionMediatingDependencySet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- Add kotlin("test") dependency to support kotlin.test.* imports in tests
- Rename BrownfieldPrimitives.kt to BrownfieldPublishingInfo.kt per ktlint single-class-per-file rule
- Fix test class formatting per ktlint standard:no-empty-first-line-in-class-body
All tests pass: 3/3 VersionMediatingDependencySetTest tests pass
Build: BUILD SUCCESSFUL
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…iance) Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…overerTest Adds a real dependency to the runtimeOnly configuration and asserts it is discovered, closing a mutation-testing gap where deleting "runtimeOnly" from configNames left the test green. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ollision Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The sentence 'Skip this task-registration block entirely...' was incorrectly placed inside the kotlin code fence. Moved it outside as a separate paragraph before the fence opens to ensure proper rendering and syntax highlighting. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Added comprehensive documentation of the critical fix that relocated the warning sentence outside the kotlin code fence to ensure proper rendering and syntax highlighting. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… POM filter Turns on the plugin's includeTransitiveDependencies option in the RNApp demo's BrownfieldLib module and deletes the hand-rolled pom.withXml / module.json post-processing task that predates this feature, now that the plugin itself strips embedded-module entries and injects real transitive dependencies.
- ci: add gradle-plugins path filter to Expo Android road-test job gates - ci: run gradle-plugins unit tests in the ktlint/detekt lint workflow - docs: split publishing/task-registration code fences so the "skip this block" note describes only the skippable part, and fix a stale below/above reference - plugin: tighten removalPredicate to require matching group AND artifact name, avoiding over-exclusion of unrelated third-party POM entries - plugin: restore diagnostic Logging.log() calls at the centralized transitive-dependency merge/injection call site - untrack accidentally-committed task-7-report.md workspace artifact and ignore .superpowers/ going forward Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… discovery ExpoPublishingHelper.appendExpoTransitiveDependenciesFromGradle enumerated "implementation", "api", "runtime" — but plain "runtime" isn't a real configuration on modern AGP/Gradle library modules (legacy Java-plugin name; the correct one is "runtimeOnly"). That leg has silently been a no-op since this code was introduced (callstack#223). Found while building the equivalent RNC-CLI discoverer for this branch, which correctly used "runtimeOnly" from the start. Fixing here as a separate, standalone bug fix rather than folding it into the feature commits — this method is only a fallback path (used when an Expo module's POM file can't be found on disk), so the blast radius is narrow, but it's a confirmed real bug worth closing while we're here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…iscovery Mutation-tested manually: fails against the pre-fix "runtime" typo, passes against the "runtimeOnly" fix from the previous commit. Ran the real ExpoApp57 build with the fix applied too -- discovered-dependency counts for the 4 modules that actually exercise this fallback path (expo, expo-constants, expo-modules-core, expo-updates) are unchanged (6/2/11/11 before and after), so the bug has no observable impact on this repo's current Expo dependency set. This test is what actually proves the fix, independent of whether any current module happens to trigger it. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…refs Extracts the Gradle-configuration-walking logic shared by the Expo Gradle-fallback and RNC-CLI discoverers into collectPublishableGradleDependencies, so the isPublishableCoordinate filter (rejecting dynamic/blank versions) now applies to both paths instead of only the RNC one. Also logs a warning when RncTransitiveDependencyDiscoverer can't resolve an embedded module's Gradle project, instead of silently skipping it, and removes code comments referencing a design-spec doc that was never committed to this branch. Verified with ktlintCheck + unit tests, and end-to-end via the RNApp -> AndroidApp vanilla Detox suite (built AAR with includeTransitiveDependencies enabled, inspected the generated POM/module.json for correct injection, all 4 Detox tests passed). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds an opt-in
includeTransitiveDependenciesoption to the Brownfield Gradle Plugin. When a vanilla (non-Expo) brownfield app turns it on, BGP discovers the real third-party dependencies of its embedded native modules and publishes them into the AAR's POM/Gradle Module Metadata, so a consuming native app resolves them automatically instead of the host team having to hand-declare them. Mirrors the mechanism Expo projects already get unconditionally.Notion ticket: Brownfield: support transitive dependencies in BGP
What's in this PR
includeTransitiveDependenciesoption onreactBrownfield { }RNAppdemo, hand-rolled workaround removedappendExpoTransitiveDependenciesFromGradlewas checking for a Gradle configuration named"runtime", which doesn't actually exist on modern AGP modules (the real name is"runtimeOnly") — so that fallback path has been silently skipping runtimeOnly dependencies since feat: expo config plugin #223. Fixed and added a regression test for it.Testing
Verified locally: real Maven-local publish for both
RNAppandExpoApp57, inspected the generated POM/module.json (confirmed a genuinely new dependency gets injected, embedded modules stay excluded, dynamic-version coordinates get filtered), built the vanillaAndroidAppflavor against it, ran the existing Detox suites for both vanilla and Expo — all green.Added unit tests for this, not the usual pattern in this repo, but they cover an actual bug I spotted and fixed in the Expo implementation so maybe they are worth to keep. Let me know what you think.