Skip to content

[eas-cli] fix capability syncing #3086

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

Merged
merged 3 commits into from
Jul 14, 2025

Conversation

vonovak
Copy link
Contributor

@vonovak vonovak commented Jul 3, 2025

Why

This PR refactors the iOS bundle ID capabilities management to improve handling of capabilities with complex settings. The current implementation doesn't properly handle capabilities that have settings - Sign In with Apple, iCloud, and Data Protection.

How

  • Added test cases that would previously fail
  • Extracted capability list to a separate file (capabilityList.ts) to improve code organization
  • Fixed synchronization logic to make tests pass (in a dirty way, refactored later)
  • Fixed handling of MDM Managed Associated Domains to prevent disabling when Associated Domains is enabled
  • Improved test assertions to use non-deprecated matchers (toHaveBeenCalledWith instead of toBeCalled)

Test Plan

  • green CI
  • local testing

Copy link
Contributor Author

vonovak commented Jul 3, 2025

Copy link

github-actions bot commented Jul 3, 2025

Size Change: -3.19 kB (-0.01%)

Total Size: 53.5 MB

Filename Size Change
./packages/eas-cli/dist/eas-linux-x64.tar.gz 53.5 MB -3.19 kB (-0.01%)

compressed-size-action

@vonovak vonovak force-pushed the vonovak/_eas-cli_improve_capability_syncing branch 4 times, most recently from 7d8d903 to aeabf23 Compare July 3, 2025 17:45
// - settings are not defined in the existing capability, but usesBroadcastPushNotifications is enabled (we want to add settings for this capability)
// - settings are defined in the existing capability, but usesBroadcastPushNotifications is disabled (we want to remove settings for this capability)
const noSettingsAttributes = existing.attributes.settings == null;
return !noSettingsAttributes === additionalOptions.usesBroadcastPushNotifications;
}

function shouldSkipIcloudCapabilityUpdate(
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is removed in the next PR up the stack so not worth spending much time here

@vonovak vonovak marked this pull request as ready for review July 4, 2025 08:13
Copy link

github-actions bot commented Jul 4, 2025

Subscribed to pull request

File Patterns Mentions
**/* @sjchmiela

Generated by CodeMention

@douglowder douglowder self-requested a review July 6, 2025 10:04
@douglowder douglowder added the no changelog PR that doesn't require a changelog entry label Jul 7, 2025 — with Graphite App
@vonovak vonovak force-pushed the vonovak/_eas-cli_improve_capability_syncing branch from aeabf23 to c736c75 Compare July 14, 2025 17:04
Copy link

codecov bot commented Jul 14, 2025

Codecov Report

Attention: Patch coverage is 95.74468% with 4 lines in your changes missing coverage. Please review.

Project coverage is 51.77%. Comparing base (d87f8d5) to head (f16fa46).
Report is 1 commits behind head on main.

Files with missing lines Patch % Lines
...c/credentials/ios/appstore/bundleIdCapabilities.ts 96.30% 2 Missing ⚠️
...cli/src/credentials/ios/appstore/capabilityList.ts 94.74% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3086      +/-   ##
==========================================
+ Coverage   51.68%   51.77%   +0.09%     
==========================================
  Files         606      607       +1     
  Lines       24344    24382      +38     
  Branches     5105     5123      +18     
==========================================
+ Hits        12580    12621      +41     
+ Misses      10713    10711       -2     
+ Partials     1051     1050       -1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vonovak vonovak changed the title [eas-cli] improve capability syncing [eas-cli] fix capability syncing Jul 14, 2025
Copy link
Contributor Author

vonovak commented Jul 14, 2025

Merge activity

  • Jul 14, 8:04 PM UTC: A user started a stack merge that includes this pull request via Graphite.
  • Jul 14, 8:05 PM UTC: Graphite rebased this pull request as part of a merge.
  • Jul 14, 8:09 PM UTC: @vonovak merged this pull request with Graphite.

@vonovak vonovak force-pushed the vonovak/_eas-cli_improve_capability_syncing branch from 4aaaa78 to f16fa46 Compare July 14, 2025 20:05
Copy link

⏩ The changelog entry check has been skipped since the "no changelog" label is present.

@vonovak vonovak merged commit 8431167 into main Jul 14, 2025
12 checks passed
@vonovak vonovak deleted the vonovak/_eas-cli_improve_capability_syncing branch July 14, 2025 20:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no changelog PR that doesn't require a changelog entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants