feat(react-native): scope event release mode to the Hermes upload - #4717
feat(react-native): scope event release mode to the Hermes upload#4717ablaszkiewicz wants to merge 7 commits into
Conversation
The Hermes maps upload release-independent by default. Two releases that ship the same JavaScript no longer both report whichever release uploaded the maps first. iOS dSYMs and Android R8 mappings always bind to the release their build creates. The symbol id there is the binary UUID or a hash of the mapping, so two releases collide only when they ship a byte-identical artifact. The dSYM path also lost release attribution for embedded targets. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jg6r6rFUBnowrXssZyU7yR
The generated dSYM phase carries no release mode, and the plugin no longer recognizes the block the previous version wrote. posthog-ios does not read POSTHOG_NO_RELEASE_BIND any more, so a stale export in an already prebuilt project sets a variable nothing reads. addDsymUploadBuildPhase and buildDsymUploadShellScript no longer take a release mode. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jg6r6rFUBnowrXssZyU7yR
🦔 PostHog Review reviewed this pull requestFound 1 must fix, 4 should fix, 0 consider. Published 5 findings (view the review). Resolved comments: 2 fixed, 3 left for you |
|
Size Change: -1.82 kB (-0.01%) Total Size: 20.8 MB 📦 View Changed
ℹ️ View Unchanged
|
|
PostHog Review alpha 🦔 If you find any issues helpful - please reply "valid", "invalid", etc., for evaluation purposes 🙏 |
…ilds A native-only compile sets SKIP_BUNDLING and exits before the hermes clone/upload calls, so the --release-mode flag those carry is never applied. With event now the default release mode, the version floor at posthog-xcode.sh fired on every unconfigured build and ran before the SKIP_BUNDLING early-exit, failing such a build over a posthog-cli version it never exercises. Gate the floor check on SKIP_BUNDLING being unset so a skipped build with an old or source-built CLI passes again, restoring the pre-feature behavior. The existing forwarding test no longer needs its POSTHOG_SKIP_CLI_VERSION_CHECK workaround, and a dedicated regression test asserts a SKIP_BUNDLING build with an old CLI succeeds and runs no hermes calls. Generated-By: PostHog Desktop Task-Id: d90a3055-12ee-4d22-bff7-6fa3fd3abde4
…tribution The releaseMode prop doc described the $app_namespace/$app_version/$app_build coordinates as ones "the SDK already sends", but the SDK reads them only from the optional expo-application or react-native-device-info module (native-deps.tsx), and a plain customAppProperties object replaces the defaults rather than merging. An install with neither module still symbolicates in event mode but its JavaScript exceptions carry no release. Document that requirement on the prop so a config author knows event-mode release attribution depends on an app-metadata source, and that symbol-set does not. The default stays event and no runtime behavior changes; the existing runtime warning already self-announces the degraded install. Generated-By: PostHog Desktop Task-Id: d90a3055-12ee-4d22-bff7-6fa3fd3abde4
The prebuild writes posthog.hermesReleaseMode, which com.posthog.android does not read, and removes any posthog.releaseMode entry an earlier version left behind. posthog.gradle reads the new key. The releaseMode prop reached only the iOS plugin, so an explicit symbol-set left the Android Hermes upload on the event default. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Jg6r6rFUBnowrXssZyU7yR
|
Reviews (1): Last reviewed commit: "fix(react-native): keep the Hermes mode ..." | Re-trigger Greptile |
marandaneto
left a comment
There was a problem hiding this comment.
Automated advisory code review.
marandaneto
left a comment
There was a problem hiding this comment.
Additional advisory review comment.
marandaneto
left a comment
There was a problem hiding this comment.
Additional advisory review comment.
…floor Review blocked three compatibility gaps. A prebuild now recognizes and rewrites the dSYM phase 4.64-4.66 generated, whose stale POSTHOG_NO_RELEASE_BIND export the pinned posthog-ios still reads, with verbatim fixtures proving the migration idempotent. The prebuild no longer deletes a user-owned posthog.releaseMode entry from gradle.properties; posthog.gradle reads it as a deprecated fallback with a warning instead. And an unconfigured build on posthog-cli < 0.16.0 warns and uploads bound instead of failing: only an explicitly configured event mode still fails the build. The prebuild writes the mode into build files only when configured, so the scripts can tell the default apart. Also keeps the EXPERIMENTAL marker on the prop and trims the changeset, per review. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0177eam86deHk8HZYMKcJtbD
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_0177eam86deHk8HZYMKcJtbD
|
|
||
| doLast { | ||
| def cliPackage = PostHogCli.resolveCliPackagePath(rootDirFile, reactRoot) | ||
| def posthogReleaseModeArgs = PostHogCli.releaseModeArgs( |
There was a problem hiding this comment.
[question] releaseModeArgs runs resolveVersion here and again in doFirst, and that spawns posthog-cli --version each time — so every Android release build now pays two extra subprocess spawns, where before the check ran once and only when someone had explicitly configured a non-default mode. Could we memoize the lookup in a static on PostHogCli keyed by the command, or does the configuration cache get in the way of that too?
| Scope `releaseMode` to the Hermes source map upload and default it to `event`. Each JavaScript exception resolves its own release from the app metadata the SDK sends, so two releases that ship the same bundle stop reporting whichever release uploaded the maps first. iOS dSYMs and Android R8 mappings always bind to the release their build creates. | ||
|
|
||
| Set `releaseMode: 'symbol-set'`, or `POSTHOG_RELEASE_MODE=symbol-set` before the prebuild, to keep stamping the release onto the maps. Event mode needs posthog-cli 0.16.0: a build that configured no mode warns and keeps binding on an older CLI, while an explicitly configured `event` fails the build and names the upgrade. A prebuild also rewrites the dSYM phase an earlier plugin version generated, removing its deprecated `POSTHOG_NO_RELEASE_BIND` export. |
There was a problem hiding this comment.
[suggestion] The rename isn't in here — a project carrying posthog.releaseMode in gradle.properties starts warning on its next build and the release notes never mention it. Worth adding, and the surrounding prose is mostly "why" that already lives in the PR body, so folding both into a Common Changelog shape:
| Scope `releaseMode` to the Hermes source map upload and default it to `event`. Each JavaScript exception resolves its own release from the app metadata the SDK sends, so two releases that ship the same bundle stop reporting whichever release uploaded the maps first. iOS dSYMs and Android R8 mappings always bind to the release their build creates. | |
| Set `releaseMode: 'symbol-set'`, or `POSTHOG_RELEASE_MODE=symbol-set` before the prebuild, to keep stamping the release onto the maps. Event mode needs posthog-cli 0.16.0: a build that configured no mode warns and keeps binding on an older CLI, while an explicitly configured `event` fails the build and names the upgrade. A prebuild also rewrites the dSYM phase an earlier plugin version generated, removing its deprecated `POSTHOG_NO_RELEASE_BIND` export. | |
| Scope `releaseMode` to the Hermes source map upload and default it to `event`; iOS dSYMs and Android R8 mappings always bind to the release their build creates. Set `releaseMode: 'symbol-set'`, or `POSTHOG_RELEASE_MODE=symbol-set` before the prebuild, to keep stamping the release onto the maps. | |
| Rename the `posthog.releaseMode` gradle property to `posthog.hermesReleaseMode`. The old key still works, with a deprecation warning. | |
| `event` mode needs posthog-cli 0.16.0: an unconfigured build warns and keeps binding on an older CLI, an explicitly configured `event` fails the build and names the upgrade. |
Related PRs
Event mode is live today, and it is opt-in. These PRs settle where it stays and where it goes.
Deprecating the mobile knobs. The symbol id on these paths is already a content hash, so two releases collide only when they ship a byte-identical artifact. An ordinary release that changes code gets its own symbol set and never collides. The dSYM path also lost release attribution for embedded targets, because one upload covers every target while it creates one release. Review asked for deprecation instead of removal, so every knob stays accepted as a warned no-op.
Making it the default.
React Native. Removes the iOS and Android propagation, and makes event mode the default for the Hermes upload. That is the one path where two releases really do ship the same artifact.
Problem
Changes
releaseModenow steers the Hermes source map upload only, and it defaults toevent.POSTHOG_RELEASE_MODEat prebuild time. With neither configured, the prebuild writes nothing and the build scripts apply the default themselves.eventfails the build and names the upgrade.POSTHOG_NO_RELEASE_BINDexport would otherwise keep steering the pinned posthog-ios, which still reads the variable until chore: deprecate POSTHOG_NO_RELEASE_BIND in the dsym upload posthog-ios#791 ships.posthog.releaseModeentry from the user'sgradle.properties. It writes the Hermes mode underposthog.hermesReleaseMode, andposthog.gradlereads the legacy key as a deprecated fallback with a warning.addDsymUploadBuildPhaseandbuildDsymUploadShellScriptno longer take a mode.releaseModeprop keeps its EXPERIMENTAL marker while the rollout settles, and the changeset shrinks to a short summary, per review.Note
Two follow-ups stay open until the sibling PRs release: bump
POSTHOG_ANDROID_GRADLE_PLUGIN_VERSIONto the posthog-android release carrying #747 (1.5.2 still readsposthog.releaseModeandPOSTHOG_RELEASE_MODEfor R8), and bump the podspec's posthog-ios pin to the release carrying #791.How did you test this code?
jestinpackages/react-native: 682 pass;test/optional-plugin.spec.tsfails in this sandbox on the unchanged branch too, so it is environmental.--release-mode. The explicit-mode floor tests keep asserting the hard failure.posthog.releaseModeentry in place while writingposthog.hermesReleaseMode.🤖 Agent context
Autonomy: Human-driven (agent-assisted)
Written with Claude Code (Opus 5); the review round (phase migration, legacy key preservation, soft CLI floor) with Claude Code (Fable 5). Skills invoked:
/writing-pr-descriptions,/writing-tests,/writing-code-comments.This replaces #4706. The review round addresses the three blocking comments: refresh the previous release's dSYM phases, deprecate
posthog.releaseModeinstead of deleting it, and cover the migration with fixtures of the published phase text. It also answers the floor question: an unconfigured project never fails its build over an old CLI.