Advertise binary diff capability to the server - #63
Merged
Merged
Conversation
ofalvai
changed the title
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
The TypeScript build failure and broken bare-iOS test setup are approval-blocking.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 2
Open (2)
What changed in this PR
Adds opt-in binary-diff capability advertising from native configuration through the acquisition SDK.
Changes:
- Advertises capabilities during update checks and deployment reports.
- Exposes delta-update settings on iOS and Android.
- Adds Expo configuration and test coverage.
| File | Summary | Finding |
|---|---|---|
test/test.ts |
Enables and verifies E2E capability advertising. | Critical: iOS setup fails because plutil -replace targets a missing plist key. |
test/template/app.json |
Enables delta updates for Expo tests. | No issue. |
test/template/android/app/src/main/res/values/strings.xml |
Enables Android test-app delta updates. | No issue. |
src/acquisition-sdk/types.ts |
Adds capability request fields. | No issue. |
src/acquisition-sdk/acquisition-sdk.ts |
Serializes and sends capabilities. | Critical: Object.entries is incompatible with the configured ES6 TypeScript library. |
src/acquisition-sdk/__tests__/capabilities.test.ts |
Adds capability serialization tests. | Nit: This suite is not compiled or executed by current test targets. |
ios/CodePush/CodePushConfig.m |
Exposes the iOS delta-update setting. | No issue. |
expo.js |
Adds Expo plugin support for delta updates. | No issue. |
android/app/src/main/java/com/microsoft/codepush/react/CodePushNativeModule.java |
Exposes Android configuration to JavaScript. | No issue. |
android/app/src/main/java/com/microsoft/codepush/react/CodePush.java |
Stores and exposes the Android setting. | No issue. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
|
|
||
| queryString += encodeURIComponent(property) + "="; | ||
| queryString += encodeURIComponent(value); | ||
| for (var [key, value] of Object.entries(request)) { |
Collaborator
Author
There was a problem hiding this comment.
Decided to split the fix into its own PR: #64
| } | ||
|
|
||
| async function setPlistBoolValue(plistPath: string, key: string, value: boolean): Promise<void> { | ||
| await promisify(childProcess.execFile)("plutil", ["-replace", key, "-bool", String(value), plistPath]); |
Collaborator
Author
There was a problem hiding this comment.
plutil -replace inserts the key when it's missing, it doesn't require it to exist. The existing CodePushDeploymentKey, CodePushServerURL, and CodePushPublicKey lines work the same way. They are not in any template either.
ofalvai
force-pushed
the
push-mnzosllxrtkx
branch
from
September 23, 2026 09:34
d8ccf24 to
73bca22
Compare
Sends the client's capabilities on both update_check (as a plain repeated capabilities=<value> query param, no bracket notation) and report_status/deploy (as a JSON array), so the server can serve v2 (bsdiff) diffs and meter their size correctly. The two calls must agree: report_status/deploy's capabilities drive the server's byte-count reconstruction independently of what update_check advertised, so omitting it there would silently over-report downloaded bytes for clients that received a v2 diff. The capability is advertised only when the app enables delta updates (CodePushEnableDeltaUpdates in Info.plist / strings.xml). The native side rejects a v2 diff when the flag is off, and the server would keep serving the same diff, so advertising it unconditionally would block that release on the device. Both platforms now expose the flag through getConfiguration(). On iOS it stays a typed property on CodePushConfig and is serialized only at the JS boundary, not stored in the untyped config dictionary. Replaces queryStringify with a toQueryString that expands array values into a repeated key. It builds the string by hand, because the URLSearchParams polyfill in RN 0.76-0.79 accepts only a plain object, which cannot hold a repeated key. The E2E test apps enable delta updates, and checkForUpdate.update asserts the capability on update_check, which covers the flag end to end (native config -> getConfiguration() -> SDK). Also adds CodePushEnableDeltaUpdates to the Expo config plugin for both platforms, since expo prebuild regenerates the native config and Expo apps had no way to set the flag. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
ofalvai
force-pushed
the
push-mnzosllxrtkx
branch
from
September 23, 2026 12:34
73bca22 to
fb6f322
Compare
miklosboros
approved these changes
Sep 24, 2026
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
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.
Why
We need to keep the client's capabilities in sync with the server response, in order to avoid receiving update payloads that the client can't handle.
What
Sends the client's capabilities on both
update_checkandreport_status/deployso the server can serve ZIPs with bsdiff patches (and meter their size correctly).As this feature is opt-in for now (
CodePushEnableDeltaUpdatesinInfo.plist/strings.xml), the capability is advertised only when the config is enabled. The native side rejects a v2 diff when the flag is off, and the server would keep serving the same diff, so advertising it unconditionally would block that release on the device.Since the capability is ultimately decided by the
CodePushEnableDeltaUpdatesconfig (controlled by the user of this SDK), we need to expose this value from the native sides to the JS side, where API calls likeupdate_checkare implemented.The E2E test apps enable delta updates, and
checkForUpdate.updateasserts the capability onupdate_check, which covers the flag end to end (native config ->getConfiguration()-> SDK).Also adds
CodePushEnableDeltaUpdatesto the Expo config plugin, sinceexpo prebuildregenerates the native config and Expo apps had no way to set the flag.Decisions
Wanted to replace the hand-rolled
queryStringify()function with URLSearchParams, but its React Native polyfill doesn't support what we need on RN 0.76-0.79, and I think those versions are worth supporting for now. See the inline comment for more details.Tests: adding full E2E test coverage for the delta update feature would be really complex, and would increase our already poor test run times, so I think unit tests are a better fit for this. Nevertheless, this PR enables
CodePushEnableDeltaUpdatesin the E2E test apps, and adds verification for theupdate_checkAPI call's capability field, so at least this part is covered when running the existing E2E tests.