Skip to content

fix(expo-plugins): add the playback service to manifests without <service> - #5098

Merged
mziolkowskii merged 10 commits into
masterfrom
fix/expo-plugins-service-and-podfile
Oct 8, 2026
Merged

mziolkowskii merged 10 commits into
masterfrom
fix/expo-plugins-service-and-podfile

Conversation

@mziolkowskii

@mziolkowskii mziolkowskii commented Sep 14, 2026 •

Copy link
Copy Markdown
Collaborator

Summary

Fixes four Expo config plugin bugs: the Android playback service was never added to a manifest without an existing <service> element, the foreground service permissions never reached AndroidManifest.xml, a partial androidExtensions object switched the omitted extension off, and writeToPodfile aborted prebuild on a Podfile it could not anchor into instead of warning. Adds enableAndroidPlaybackService (default true) so an app that uses neither playInBackground nor showNotificationControls can opt out of the service and its permissions. Adds unit tests for the config plugins against parsed native files, and documents on the Expo page what the plugin changes on each platform.

Split out of #5087; the unit test tooling it relies on landed with #5096.

Motivation

  • withAndroidNotificationControls pushed onto mainApplication.service with optional chaining, so a manifest with no <service> element yet (the default Expo template) silently got nothing: no VideoPlaybackService, and notification controls did not work. v6 (withNotificationControls) created the array when missing; v7 lost that.
  • The plugin added FOREGROUND_SERVICE and FOREGROUND_SERVICE_MEDIA_PLAYBACK to config.android.permissions from inside its manifest mod. expo prebuild registers its built-in mods after the app's plugins and mods run last-registered first, so Expo's own permissions mod has already written the manifest when this mod changes the config: the permissions never reached AndroidManifest.xml. v6 added them through withPermissions at plugin level, which works; this is a regression from v6. With the service now present, an app using showNotificationControls or playInBackground would start a foreground service without the permission it needs (Android 9+, and the mediaPlayback type on Android 14+).
  • withAndroidExtensions filled a key missing from a partial androidExtensions object with false, so { useExoplayerHls: false } also disabled DASH, against the documented @default true per key (same in v6).
  • mergeContents throws when its anchor is missing instead of reporting didMerge: false, so the warning branch in writeToPodfile was unreachable and prebuild aborted on a Podfile without platform :ios. Note for reviewers: writeToPodfile has no caller in v7 yet (the v6 withCaching/withAds plugins were not ported); the fix keeps the helper correct for when they are, and is covered by tests.

Changes

  • withAndroidNotificationControls: create the service array when missing, skip the service when it is already declared, and add the two permissions through AndroidConfig.Permissions.withPermissions at plugin level (as v6 did): it updates config.android.permissions when the plugin is applied, so Expo's permissions mod writes them whatever the mod order, and registers a manifest mod of its own. The warning for a manifest without a main application now names <application> instead of .MainActivity.
  • withReactNativeVideo: new enableAndroidPlaybackService option, default true; false skips the service and the permissions.
  • withAndroidExtensions: a missing key keeps its default (true).
  • writeToPodfile: a missing anchor (ERR_NO_MATCH) is a one-line warning naming the anchor; any other error (an unwritable Podfile) still aborts prebuild.
  • Unit tests for withBackgroundAudio, withAndroidPictureInPicture, withAndroidExtensions, withAndroidNotificationControls (asserting the manifest and the config, including with Expo's own permissions mod registered after the plugin, as prebuild does), writeToPodfile and withReactNativeVideo.
  • docs/.../with-expo.md: the new option and a "What the plugin changes" section per platform. The agent skill's Android background note pointed at enableBackgroundAudio, which is iOS-only; it now names the service and the new option. enableBackgroundAudio was described as an Android feature; it only edits UIBackgroundModes in Info.plist.
  • config/bun-test.d.ts: test.skipIf for the read-only Podfile case, which root would not fail.

Design note: the service and its two permissions have been added to every app that lists the plugin since #4754 (v6 gated it behind enableNotificationControls, which also missed playInBackground). That stays the default: the player starts the service at runtime when playInBackground or showNotificationControls is set, neither is known at prebuild time, and a missing service fails at runtime in a way that is hard to diagnose. Apps that use neither, and want to avoid the Play Console foreground-service declaration, opt out with enableAndroidPlaybackService: false.

Platforms affected

  • Android
  • iOS
  • visionOS
  • tvOS / Android TV
  • Windows
  • Web

Type of change

  • Bug fix (non-breaking change that fixes an issue)
  • New feature (non-breaking change that adds functionality)
  • Breaking change (fix or feature that changes existing behavior)
  • Documentation only

Test plan

  • bun run test: 223 tests pass (12 new library tests in this PR, the registration test now also covers the opt-out). bun typecheck and bun lint pass.
  • Against the plugin source on master, 5 of the new tests fail: the service and permissions on a template manifest, the service on a manifest without <service>, no duplicate service or permissions, the partial androidExtensions default, and a Podfile without an anchor.
  • The mod order was also checked outside the unit tests, by running the registered manifest mod chain with @expo/config-plugins 10.1.2 on a template-like manifest with Expo's permissions mod registered after this plugin: on master neither the service nor the permissions end up in the manifest; on this branch both are present exactly once.
  • No Maestro flow: the E2E test app uses its own config plugin, not the library's Expo plugins, and was not run through expo prebuild.

Merge order

#5097 rewrites the same line of CONTRIBUTING.md that this PR used to touch, so this PR no longer edits it; either order merges cleanly.

Checklist

  • I read the contributing guidelines.
  • I added or updated tests that cover this change, or explained under "Test plan" why none apply.
  • For a bug fix the E2E suite can exercise, this PR includes a Maestro flow that fails without the fix.
  • bun run test, bun lint and bun typecheck pass locally.
  • I updated the documentation / README where relevant.
  • If this changes how the library is used (API, props, events, behavior), I updated the AI agent skill (skills/react-native-video/) to match.
  • This PR is focused on a single concern.
  • I tested my changes on at least one platform.

Okelm pushed a commit that referenced this pull request Sep 16, 2026
… RN 0.87 (#5096)

Adds a deterministic end-to-end harness and the CI around it, plus the
iOS fix needed to build on React Native 0.87's prebuilt core.

- test-app/: react-native-test-app host driven by deep links
  (rnvtest://scenario/<name>); player events rendered as text markers
  with stable testIDs. Two-line RNTA 5.4.9 patch (CLEAR_TOP | SINGLE_TOP
  on the deep-link redirect), see test-app/patches/README.md.
- e2e/: 10 Maestro smoke flows, shared launch/open/wait-for-end subflows,
  local media fixtures served by a dependency-free server with Range
  support, per-RN-version overlays and lockfiles (0.82, 0.87; 0.77 is
  the floor).
- CI: unit.yml (lint, typecheck, unit tests on React 18; library tests
  and typecheck on React 19), e2e.yml PR gate (Android 0.77/0.82/0.87 on
  API 36, iOS 0.87 on iOS 26), e2e-nightly.yml (15-leg grid plus
  quarantine, one issue per failing row, history on the e2e-results
  branch), weekly lockfile refresh, daily cache cleanup, actionlint.
  Third-party actions pinned to SHAs; Maestro and actionlint installed
  from checksummed release archives. No check is required yet.
- Library: ReactNativeVideo.podspec no longer publishes
  Video-Bridging-Header.h (file removed); three React imports are
  framework-style. First unit tests (VideoError, useManagedInstance),
  type-checked and linted through tsconfig.test.json.
- Docs: CONTRIBUTING "Testing your change" and CI sections, PR template,
  e2e/CONTEXT.md (D1-D7), e2e/CI_MATRIX_DESIGN.md (D8-D13, one-time setup).

Stacked follow-ups: #5097 (core fixes), #5098 (Expo config plugin fixes).

Fixes #5084
Closes #5085

Co-authored-by: GratwickEnt <178044584+GratwickEnt@users.noreply.github.com>
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
…vice>

withAndroidNotificationControls pushed onto mainApplication.service with
optional chaining, so a manifest that had no <service> element yet (the default
Expo template) silently got nothing: no VideoPlaybackService, and notification
controls did not work. Create the array, skip the service if it is already
declared, and make sure the permissions array exists before pushing.

writeToPodfile: mergeContents throws when its anchor is missing instead of
reporting didMerge: false, so the warn branch was unreachable and prebuild
aborted on a Podfile without `platform :ios`. Catch and warn as intended.
Runs each registered mod the way prebuild does, with a parsed Info.plist,
AndroidManifest, gradle.properties or Podfile as modResults: background audio
on/off, Picture-in-Picture on .MainActivity, ExoPlayer extension flags with
defaults and stale-entry replacement, the playback service and permissions
(including a manifest with no <service> element and a repeated run),
writeToPodfile anchors for Expo and react-native-test-app, and the plugin
composition per prop.
@mziolkowskii
mziolkowskii force-pushed the fix/expo-plugins-service-and-podfile branch from 8be8e37 to bbeda78 Compare September 17, 2026 09:26
@mziolkowskii
mziolkowskii changed the base branch from feat/e2e-infra to master September 17, 2026 09:26
mziolkowskii and others added 2 commits September 17, 2026 11:35
master now type-checks and lints __tests__ (#5096), which these tests predate.
Extend the bun:test typings with mock.module, resolves, toBeDefined,
toBeGreaterThan/toBeLessThan and toThrow(ErrorClass), and apply Prettier.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
@Okelm
Okelm self-requested a review October 6, 2026 13:23
…manifest

The plugin added FOREGROUND_SERVICE and FOREGROUND_SERVICE_MEDIA_PLAYBACK to
config.android.permissions from inside its AndroidManifest mod. expo prebuild
applies the app's plugins before it registers its built-in ones, and mods run
starting from the last registered, so Expo's own permissions mod has already
written the manifest when this mod changes the config: the permissions never
reached AndroidManifest.xml. Now that the service is added to the default
template, an app using showNotificationControls or playInBackground would get
the service without the permissions it needs to run in the foreground
(Android 9+, and the media playback type on Android 14+).

The permissions are now added to the manifest itself with
AndroidConfig.Permissions.ensurePermissions, which does not add them twice,
and the tests assert the manifest rather than the config.

Verified by compiling the manifest mods with @expo/config-plugins 10.1.2 on
Expo's bare template, with this plugin registered before and after the
permissions mod: the service and both permissions are present exactly once.
…n defaults

Review follow-ups on this PR:

- withAndroidNotificationControls: use AndroidConfig.Permissions.withPermissions
  at plugin level instead of ensurePermissions inside the manifest mod. It adds
  the permissions to config.android.permissions when the plugin is applied, so
  Expo's own permissions mod writes them whenever it runs, registers a manifest
  mod of its own, and keeps `expo config` truthful; this is also what v6 did.
  The warning for a manifest without a main application named the wrong
  element (.MainActivity, copied from the PiP plugin).
- withAndroidExtensions: a key left out of a partial object now keeps its
  documented default (true) instead of switching the extension off, so
  { useExoplayerHls: false } no longer disables DASH as well.
- writeToPodfile: only a missing anchor (ERR_NO_MATCH) is downgraded to a
  one-line warning naming the anchor; any other error (an unwritable Podfile)
  still aborts prebuild.
- Tests: the config type comes from ConfigPlugin instead of a direct
  @expo/config-types import the package does not depend on; the test-app
  Podfile case guards against indexOf returning -1, which made it pass
  vacuously; new case for a Podfile that cannot be written.
@mziolkowskii
mziolkowskii marked this pull request as ready for review October 7, 2026 10:06
The page did not say that the plugin always registers the Android playback
service and its two foreground permissions, nor what each option writes.
enableBackgroundAudio was described as an Android feature; it only edits
UIBackgroundModes in Info.plist, Android uses the playback service instead.
…case as root

Registers Expo's own permissions mod after the plugin, as prebuild does, and
runs the manifest mod chain: the permissions must reach the manifest although
Expo's mod runs first. The read-only Podfile case is skipped under root, which
ignores file permissions. The CONTRIBUTING.md sentence is left to #5097, which
rewrites the same line; keeping both edits would conflict at merge.
…e playback service

The plugin has registered VideoPlaybackService and the FOREGROUND_SERVICE /
FOREGROUND_SERVICE_MEDIA_PLAYBACK permissions for every app since #4754 (it
only looked otherwise because neither reached the manifest; fixed earlier in
this PR). That stays the default: the player starts the service at runtime
when playInBackground or showNotificationControls is set, which prebuild
cannot know. An app that uses neither can now pass
enableAndroidPlaybackService: false to keep the service and permissions out
of its manifest, e.g. to avoid the foreground service declaration in the
Play Console. Documented on the Expo page and in the agent skill, where the
Android background note wrongly pointed at enableBackgroundAudio (iOS-only).
}

config = withAndroidNotificationControls(config);
if (props.enableAndroidPlaybackService !== false) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

It is true by default, right? otherwise this might be incorrect

@mziolkowskii
mziolkowskii merged commit 6ddd514 into master Oct 8, 2026
10 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Development

Successfully merging this pull request may close these issues.

3 participants