Skip to content

fix: handle spm packages internal to cli - #6098

Merged
edusperoni merged 4 commits into
mainfrom
chore/remove-trapeze
Jul 30, 2026
Merged

fix: handle spm packages internal to cli#6098
edusperoni merged 4 commits into
mainfrom
chore/remove-trapeze

Conversation

@NathanWalker

@NathanWalker NathanWalker commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

Swap third party trapeze dependency with cli internal handling.

  • lightens external dependency weight as well

Summary by CodeRabbit

  • New Features
    • Enhanced iOS Swift Package Manager integration for remote and local packages.
    • Swift version specifiers are now translated into Xcode-compatible requirements, with correct pbxproj quoting.
    • Packages can be linked per target when the target’s Frameworks phase is available.
  • Bug Fixes
    • pbxproj updates are now written only when packages are actually linked.
    • Improved idempotence and correct handling of same-named products across different packages.
    • Added safer behavior for missing targets/build phases (no unintended changes).
  • Refactor
    • Reworked iOS SPM application to use direct pbxproj editing via a dedicated integration service.
  • Tests
    • Expanded coverage for linking behavior, failures, quoting rules, and idempotent updates.

Swap third party trapeze dependency with cli internal handling.
@NathanWalker
NathanWalker requested a review from edusperoni July 29, 2026 20:57
@coderabbitai

coderabbitai Bot commented Jul 29, 2026

Copy link
Copy Markdown

Review Change Stack

Important

Review skipped

Review was skipped due to path filters

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json

CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including **/dist/** will override the default block on the dist directory, by removing the pattern from both the lists.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 55a00193-57e1-4dd7-99cb-70ae04a043e9

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
📝 Walkthrough

Walkthrough

SPM package models now distinguish remote and local packages. A new injected service directly edits project.pbxproj, while iOS services create target assignments and delegate package application. Tests cover version mapping, package linking, idempotence, partial failures, quoting, and invalid project roots.

Changes

SPM pbxproj integration

Layer / File(s) Summary
Package contracts and injector wiring
lib/definitions/ios.d.ts, lib/bootstrap.ts
Adds explicit remote/local package types, target assignments, the pbxproj service interface, and injector registration.
pbxproj mutation service
lib/services/ios/spm-pbxproj-service.ts
Adds direct project.pbxproj updates for package references, library products, target build phases, version requirements, quoting, and idempotent replacements.
SPM application integration
lib/services/ios/spm-service.ts, lib/services/ios-watch-app-service.ts, package.json
Builds target assignments, delegates application to the injected service, removes the former project dependency, and adds a minimatch override.
pbxproj service validation
test/services/ios/spm-pbxproj-service.ts
Tests version classification, remote/local packages, idempotence, updates, duplicate products, missing targets or phases, quoting, and invalid roots.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant SPMService
  participant SPMPbxprojService
  participant XcodeProject
  SPMService->>SPMPbxprojService: addPackages(projectRoot, assignments)
  SPMPbxprojService->>XcodeProject: parse project.pbxproj
  SPMPbxprojService->>XcodeProject: update package references and build phases
  SPMPbxprojService-->>SPMService: return applied status
Loading

Possibly related PRs

Suggested reviewers: edusperoni

Poem

I’m a rabbit with packages tucked in my sack,
Editing pbxproj without looking back.
Remote or local, each target knows,
Idempotent links bloom where dependency grows.
Tests watch the paths, versions, and phase—
Hop, hop, hooray for cleaner SPM ways!

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title matches the main change: moving SPM package handling into the CLI and removing the external Trapeze dependency.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 4

🤖 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.

Inline comments:
In `@lib/services/ios/spm-pbxproj-service.ts`:
- Around line 193-232: The product dependency and build file entries in the
package-library loop must be keyed by both owning package and product name, not
the bare library comment. Update the identifiers passed to addOrUpdateEntry for
XCSwiftPackageProductDependency and PBXBuildFile, including corresponding
comments or lookup keys, so different packages exposing the same lib name
receive distinct UUIDs without overwriting package fields; apply the same
correction to the analogous logic around the later referenced block.
- Around line 39-81: Update addPackageToTarget and addPackages so success
reflects actual project mutations: have addPackageToTarget return a boolean,
return false before any packageReferences or packageProductDependencies
initialization when the target lacks a Frameworks build phase, and return true
only after linking succeeds. In addPackages, set added from the helper’s result
so the project is written and true is returned only when at least one package
was applied.
- Around line 168-176: The requirement value assigned in the
XCRemoteSwiftPackageReference branch is pre-escaped incorrectly. Update the
packageReferenceSectionContent construction around classifyVersion() so
requirement is written through the native pbxWriter.writeObject() path, or
transformed using the writer’s expected escaping format, while preserving the
plain classified version semantics and avoiding JSON.stringify().

In `@test/services/ios/spm-pbxproj-service.ts`:
- Around line 239-271: Update the package application helper used by addPackages
to return whether it actually applied the package, including false when the
existing target lacks a Frameworks build phase. In addPackages, assign added
only from that helper’s successful result so skipped packages do not mark the
batch as added or trigger pbxproj writes; preserve successful behavior for
targets with a valid Frameworks phase.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 0a342267-1952-40cc-97ea-26ecb6911484

📥 Commits

Reviewing files that changed from the base of the PR and between 1eae2f6 and 3d797e9.

⛔ Files ignored due to path filters (1)
  • package-lock.json is excluded by !**/package-lock.json
📒 Files selected for processing (7)
  • lib/bootstrap.ts
  • lib/definitions/ios.d.ts
  • lib/services/ios-watch-app-service.ts
  • lib/services/ios/spm-pbxproj-service.ts
  • lib/services/ios/spm-service.ts
  • package.json
  • test/services/ios/spm-pbxproj-service.ts
💤 Files with no reviewable changes (1)
  • package.json

Comment thread lib/services/ios/spm-pbxproj-service.ts
Comment thread lib/services/ios/spm-pbxproj-service.ts
Comment thread lib/services/ios/spm-pbxproj-service.ts
Comment thread test/services/ios/spm-pbxproj-service.ts
NathanWalker and others added 3 commits July 29, 2026 14:18
The lockfile in this branch was produced by a full `npm install`, which
refreshed ~95 unrelated transitive versions (@babel/*, @sigstore/*,
conventional-changelog-*, and @emnapi/* jumping to 2.0.0-alpha.3) and
reshuffled hoisting, making the dependency change unreviewable.

Regenerated from main's lockfile with `npm install --package-lock-only`,
so the diff now contains only the removal of @nstudio/trapezedev-project
with its orphaned transitives and the effect of the new
@ts-morph/common@0.26.1 minimatch override.
@edusperoni
edusperoni merged commit 745f3a8 into main Jul 30, 2026
12 checks passed
@edusperoni
edusperoni deleted the chore/remove-trapeze branch July 30, 2026 00:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants