Conversation
…aunch asset (#49470) # Why An update row can end up with status READY but no launch asset row. Registration is now transactional (#49130) so new rows can no longer be committed in this state, but rows corrupted on older SDKs persist through library upgrades, and any future cause would land in the same state. Today such a row is selected for launch, fails with "Launch asset not found for update" on every cold start, and never heals: the loader short-circuits on READY so it never re-registers the assets, and the launcher keeps selecting the same broken row. This brings back the changes in #48733 by @martintreurnicht, which was closed in favor of #49130. # How - DatabaseLauncher.getLaunchableUpdate skips any non-DEVELOPMENT update whose launch asset row is missing and logs a warning. Selection can then fall back to an older complete update, the embedded update, or nothing, instead of a row that can never launch. - Loader.processUpdate's READY short-circuit now also requires the launch asset to exist. A broken row falls through to downloadAllAssets, which re-registers the assets and repairs it on the next load pass The launcher guard is the safety net until repair happens, and the loader guard is the repair path. DEVELOPMENT rows are exempt because they legitimately have no asset rows. # Test Plan Added new tests
…n in Loader (#49471) # Why #43966 fixed lost asset writes by making Loader's bookkeeping collections thread safe. That race silently dropped assets from updates that were then marked READY, which bricks the app at launch, and nothing in the test suite would catch a regression: the existing loader tests run on a single-threaded test dispatcher, so the download completions can never overlap. # How Adds LoaderStressTest, which loads a generated manifest of 100 assets plus the launch asset through a real RemoteLoader on a Dispatchers.IO scope, with small random delays in the mocked downloads so completions genuinely interleave. It runs five iterations against fresh in-memory databases and asserts the update is READY, has its launch asset, and has every asset row registered. # Test Plan With #43966's synchronization temporarily reverted to plain collections, the test fails on the first iteration with a ConcurrentModificationException out of notifyAssetLoadProgress. With the synchronization in place, all iterations pass.
#49234) # Why `File.readableStream()` returns zeroed bytes, and silently corrupts the caller's buffer, when it is read with a BYOB reader whose view starts at a non-zero offset. `byobRequest.view` covers the region of the caller's buffer that the stream still has to fill. `byteLength` is already that region's size, and `byteOffset` is where it begins in the buffer. [`FileSystemReadableStreamSource.pull`](https://github.com/expo/expo/blob/main/packages/expo-file-system/src/internal/streams.ts#L28-L41) treated both as if they were measured from the start of the buffer: ```ts const bytes = await this.handle.readBytes(theView.byteLength - theView.byteOffset); ... if (theView instanceof Uint8Array) { theView.set(bytes, theView.byteOffset); } else { const array = new Uint8Array(theView.buffer); for (let i = 0; i < bytes.length; i++) { array[i + (theView.byteOffset ?? 0)] = bytes[i]!; } } ``` Two defects: 1. `readBytes(byteLength - byteOffset)` under-reads by `byteOffset`, and asks for a non-positive length once `byteOffset >= byteLength`. 2. `theView.set(bytes, theView.byteOffset)` — `set` takes an offset **relative to the view**, and the view already starts at `byteOffset`, so the offset is applied twice. The two branches of that `if` disagree with each other. The `else` branch indexes a whole-buffer `Uint8Array` by `i + byteOffset`, which is correct. Only the `Uint8Array` branch double-applies. `byobRequest.view` has a non-zero `byteOffset` whenever the caller passes an offset view, and also on any continuation of a partially-filled BYOB read, since the spec builds the view as `(buffer, byteOffset + bytesFilled, byteLength - bytesFilled)`. # How `byteLength` is the amount to read, and the write goes through a `Uint8Array` bounded to the view's region: ```ts const bytes = await this.handle.readBytes(theView.byteLength); ... new Uint8Array(theView.buffer, theView.byteOffset, theView.byteLength).set(bytes); ``` That is correct for every view type, so the `instanceof` branch goes away. The `TODO` above it still stands — a native method writing straight into the view at an offset would avoid this copy. Partial reads are unaffected: a short `bytes` writes only what it has, and `respond(bytes.length)` is unchanged. # Test Plan New unit tests in `src/internal/__tests__/streams-test.ts`. They drive `pull()` with a `byobRequest` stand-in, because jsdom has no `ReadableStream` and the Web jest project would otherwise fail with `ReferenceError: ReadableStream is not defined`. Before, on `main`: ``` Tests: 12 failed, 12 passed, 24 total Test Suites: 4 failed, 4 total ``` The three failing cases, in all four projects: ``` ✕ fills a BYOB view that starts at a non-zero offset in its buffer ✕ requests as many bytes as the BYOB view can hold ✕ fills a BYOB view that is not a Uint8Array ``` After: ``` PASS Node | PASS Web | PASS Android | PASS iOS Tests: 24 passed, 24 total ``` I also checked this end to end against Node's real `ReadableStream`, reading a file of `1..64` through a BYOB reader with `new Uint8Array(new ArrayBuffer(32), 8, 16)`. `main`: ``` value bytes : [0,0,0,0,0,0,0,0] expected : [1,2,3,4,5,6,7,8] whole buffer : [0,0,0,0,0,0,0,0, 0,0,0,0,0,0,0,0, 1,2,3,4,5,6,7,8, 0,0,0,0,0,0,0,0] ``` The caller gets eight zero bytes, and the file data is written at offset 16 — past the range `respond()` reported as filled, so it also clobbers whatever the caller had after the view. This PR: ``` value bytes : [1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16] expected : [1,2,3,4,5,6,7,8,9,10,11,12,13,14,15,16] whole buffer : [0,0,0,0,0,0,0,0, 1,2,...,16, 0,0,0,0,0,0,0,0] ``` `et check-packages expo-file-system` -> `🏁 All checks passed`. # Checklist - [x] Added a `CHANGELOG.md` entry. - [x] Added tests that fail on `main` and pass here. - [x] Conforms to the [documentation writing style guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md).
…9481) > [!WARNING] > **Agent-authored and NOT human-reviewed.** An automated `/verify --fix` run for #49468 wrote this change and checked it in a sandbox; the reasoning and evidence are in the outcome comment on that issue. Review it as you would any external contribution. Requested by @brentvatne · [investigation run](https://github.com/expo/expo/actions/runs/33140294393) · refs #49468 The Android code reads the manifest key `expo.modules.notifications.large_notification_icon` and passes it to `setLargeIcon`. The config plugin never writes that key. A project therefore cannot set a notification large icon, and a `largeIcon` key in `app.json` is dropped without a warning. A `TODO` marks the gap. This adds a `largeIcon` property. When it is set, prebuild generates `notification_large_icon.png` for five densities and adds the meta-data entry. With `largeIcon` unset, the prebuilt manifest and every generated drawable are byte-identical to the current release. One case does change: the plugin now clears that key when `largeIcon` is unset, so a custom plugin that writes it must be listed before `expo-notifications`. Both are measured below. <details><summary>Cause</summary> The Android runtime side is complete. `ExpoNotificationBuilder` defines the key, reads it from `ApplicationInfo.metaData`, and applies it: - key: [`ExpoNotificationBuilder.kt` L402-L403](https://github.com/expo/expo/blob/f423e2ae75422c2860cee500b13cee695dbbe558/packages/expo-notifications/android/src/main/java/expo/modules/notifications/notifications/presentation/builders/ExpoNotificationBuilder.kt#L402-L403) - read: [`ExpoNotificationBuilder.kt` L319-L340](https://github.com/expo/expo/blob/f423e2ae75422c2860cee500b13cee695dbbe558/packages/expo-notifications/android/src/main/java/expo/modules/notifications/notifications/presentation/builders/ExpoNotificationBuilder.kt#L319-L340) - use: [`ExpoNotificationBuilder.kt` L150-L155](https://github.com/expo/expo/blob/f423e2ae75422c2860cee500b13cee695dbbe558/packages/expo-notifications/android/src/main/java/expo/modules/notifications/notifications/presentation/builders/ExpoNotificationBuilder.kt#L150-L155) The plugin side is not. `setNotificationConfig` writes only the small icon, the color and the default channel: [`withNotificationsAndroid.ts` L119-L173](https://github.com/expo/expo/blob/f423e2ae75422c2860cee500b13cee695dbbe558/packages/expo-notifications/plugin/src/withNotificationsAndroid.ts#L119-L173). `withNotificationsAndroid` destructures a fixed set of properties, so `largeIcon` in `app.json` reaches nothing: [`withNotificationsAndroid.ts` L261-L270](https://github.com/expo/expo/blob/f423e2ae75422c2860cee500b13cee695dbbe558/packages/expo-notifications/plugin/src/withNotificationsAndroid.ts#L261-L270). The `TODO` sits at [L48-L49](https://github.com/expo/expo/blob/f423e2ae75422c2860cee500b13cee695dbbe558/packages/expo-notifications/plugin/src/withNotificationsAndroid.ts#L48-L49). [#10492](#10492) removed large icon support from the managed workflow in 2020, in `ScopedExpoNotificationBuilder.java`. The native reader stayed. The config side was never added back. The 64 dp baseline matches Android's own `notification_large_icon_width` and `notification_large_icon_height`. The small icon keeps its 24 dp baseline. </details> <details><summary>Verification</summary> A fresh `create-expo-app` project on `expo@57.0.17`, `expo-notifications@57.0.15`, `react-native@0.86.3`. Every arm is `npx expo prebuild -p android --clean` with the image cache cleared first. The "this change" arms use `plugin/build/withNotificationsAndroid.js` from this repository's own `pnpm run build` (`sha256:24eb3041…`), copied into the app's `node_modules`. The "published" arms use `sha256:ae1bc74d…`, which matches a fresh `npm i expo-notifications@57.0.15`. | Arm | Plugin | `largeIcon` in app.json | meta-data | drawables | |---|---|---|---|---| | Before | published 57.0.15 | yes | absent | none | | After | this change | yes | present | 5 densities | | Guard | this change | no | absent | none | The after arm adds exactly one line to the generated manifest: ``` <meta-data android:name="expo.modules.notifications.large_notification_icon" android:resource="@drawable/notification_large_icon"/> ``` Generated drawables in the after arm. The small icon is unchanged: ``` drawable-mdpi/notification_icon.png 24x24 drawable-hdpi/notification_icon.png 36x36 drawable-xhdpi/notification_icon.png 48x48 drawable-xxhdpi/notification_icon.png 72x72 drawable-xxxhdpi/notification_icon.png 96x96 drawable-mdpi/notification_large_icon.png 64x64 drawable-hdpi/notification_large_icon.png 96x96 drawable-xhdpi/notification_large_icon.png 128x128 drawable-xxhdpi/notification_large_icon.png 192x192 drawable-xxxhdpi/notification_large_icon.png 256x256 ``` Guard arm, `largeIcon` unset, published against this change: ``` diff manifest-baseline.xml manifest-guard.xml -> MANIFEST IDENTICAL diff res-baseline.txt res-guard.txt -> DRAWABLES IDENTICAL ``` `res-*.txt` are `sha256sum` lists of every generated `notification*` drawable, so the bytes are identical, not only the file names. </details> <details><summary>The behaviour that does change</summary> The guard arm only covers a project that never had the meta-data key. This change adds an `else` branch that removes `expo.modules.notifications.large_notification_icon` and deletes `notification_large_icon.png` when `largeIcon` is unset. Today the plugin never touches that key or that filename, so a project that writes it from a custom plugin is a separate case. That is a real population, because a custom plugin is currently the only way to set a large icon. All four combinations, with `largeIcon` unset in `app.json` in every row: | Plugin | Custom plugin listed | meta-data entries | `notification_large_icon.png` files | |---|---|---|---| | published 57.0.15 | before `expo-notifications` | 1 | 5 | | published 57.0.15 | after `expo-notifications` | 1 | 5 | | this change | before `expo-notifications` | 1 | 5 | | this change | after `expo-notifications` | 0 | 0 | A plugin listed later has its mods run earlier, so the custom plugin writes first and the `expo-notifications` mod then clears the result. Listing the custom plugin before `expo-notifications` is safe with both versions. This matches how the plugin already treats `icon`, `color` and `defaultChannel`, which it also clears when unset. The difference is that no third party writes those keys. If you would rather not clear this key at all, option 2 in the block below is the smaller change. </details> <details><summary>Checks run</summary> From `packages/expo-notifications` in a full `pnpm install` of this repository at the checkout commit: ``` pnpm run typecheck -> exit 0 pnpm run lint -> Found 0 warnings and 0 errors (115 files) pnpm test -> 13 suites, 69 tests, 1 snapshot, all passing pnpm run build -> exit 0 pnpm run depscheck -> exit 0 npx oxfmt --check -> All matched files use the correct format ``` The test count includes two new plugin tests: one for the five generated large icon files, one for safe and idempotent removal. This change also touches one documentation page, so the `docs-pr` checks ran from `docs/`: ``` pnpm test -> 58 suites, 623 tests, 32 snapshots, all passing pnpm lint-prose -> 0 errors, 0 warnings, 0 suggestions in 1600 files pnpm lint -> oxfmt, tsc and eslint pass; oxlint crashed ``` The `oxlint` step crashed with a Rust panic in `oxc_allocator`. It crashes the same way on the unmodified tree in the same environment, so this change did not cause it. </details> <details><summary>Not covered</summary> - No Android emulator or device run. This change only produces build inputs. The native code that draws the large icon is unchanged. - No bare project with a hand-edited `android/AndroidManifest.xml` that prebuild does not regenerate. Every arm ran `prebuild --clean`. - `largeIcon` is not added to the versioned SDK 57 documentation pages, only to `unversioned`. - A notification that carries its own image still wins over this icon, because `ExpoNotificationBuilder` prefers `notificationContent.getImage`. That behaviour is unchanged. </details> <!-- expo-bot:fix-options v1 --> <details><summary>Options considered</summary> 1. **Do nothing and document that the large icon needs a custom config plugin.** Costs nothing in code, but leaves a `TODO` that names this exact work, and leaves every project writing the same plugin by hand. Rejected: the native reader already exists, so only the config side is missing. 2. **Add the property but never clear the key when `largeIcon` is unset.** Drops the `else` branch, so the last row of the table above would read 1 and 5, and no custom plugin could be broken by ordering. Cost: it breaks symmetry with `icon`, `color` and `defaultChannel`, and a bare project that removes `largeIcon` from `app.json` keeps a stale entry and a stale drawable until a clean prebuild. Rejected on that inconsistency, but it is the option to pick if the ordering interaction is judged worse. 3. **Accept a drawable resource name instead of an image path, for example `"largeIcon": "@drawable/my_icon"`.** Smaller, but the user must place the drawable in `android/` by hand, and prebuild regenerates that directory in both the managed and the bare workflow. Rejected: the file would not survive the next prebuild. 4. **Add a `largeIcon` property that generates the drawables and writes the meta-data, mirroring the existing `icon` path.** Chosen: it reuses the small icon's own generate-and-write code, it is symmetric with `icon` for users, and the prebuild output is byte-identical for projects that do not set it and do not write the key themselves. </details> <!-- /expo-bot:fix-options --> --------- Co-authored-by: expo-bot <expo-bot@users.noreply.github.com>
# Why In many places we still read `getState` or `store.state` instead of relying on the global state passed to navigators. # How 1. Remove `getState` and `getStateForKey`, and use `RootNavigationStateContext` and `NavigatorStateContext` instead 2. Replace usages of `store.state` by `RootNavigationStateContext` # Test Plan CI # Checklist <!-- Please check the appropriate items below if they apply to your diff. --> - [ ] I added a `changelog.md` entry and rebuilt the package sources according to [this short guide](https://github.com/expo/expo/blob/main/CONTRIBUTING.md#-before-submitting) - [ ] This diff will work correctly for `npx expo prebuild` & EAS Build (eg: updated a module plugin). - [ ] Conforms with the [Documentation Writing Style Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
## Why The `stackRef` is one of the blockers for concurrent react migration. It is used only in react-navigation/devtools which will no longer work with expo-router anyway. We will add our own devtools as part of https://linear.app/expo/issue/ENG-20826 ## How 1. Remove `stackRef` and `withStack` 2. Remove `stack` param from `__unsafe_action__` --------- Co-authored-by: Expo Bot <34669131+expo-bot@users.noreply.github.com>
# Why Remove the global imperative `store` # How 1. Replace usages of `store.linking` and `store.routeNode` with in-tree RouterConfig context 2. Remove `store` 3. Fix tests 4. Replace `store.state` with `navigationRef.getRootState()` # Test Plan CI # Checklist <!-- Please check the appropriate items below if they apply to your diff. --> - [ ] I added a `changelog.md` entry and rebuilt the package sources according to [this short guide](https://github.com/expo/expo/blob/main/CONTRIBUTING.md#-before-submitting) - [ ] This diff will work correctly for `npx expo prebuild` & EAS Build (eg: updated a module plugin). - [ ] Conforms with the [Documentation Writing Style Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
# Why When refactoring router to use global state - #49297 - a regression was introduced - prevent remove and action listeners stopped working. This PR fixes that. # How 1. When reducing in global state reducer return not only state but also events - `{ state, events }`. Events can be of different types (three right now - `action-dispatched`, `route-removed`, `remove-prevented`). 2. After next render (in `useLayoutEffect`) `useNavigationTreeReportEvents` processes these events and emits the react-navigation ones. After processing it dispatches `REPORT_CONSUMED` action to reducer to remove the processed events (by id) 3. `usePreventRemove` is refactored to align better with current architecture. It uses context to pass a set of prevented ids to the reducer. This context stores id of the route with `usePreventRemove(true)` and all of its parents. Since all the synchronization between the context passed to reducer and `usePreventRemove` is executed in effect phase, the hook returns a function which disables prevention synchronously - for the next reducer render, rather then waiting one commit. # Test Plan CI # Checklist <!-- Please check the appropriate items below if they apply to your diff. --> - [ ] I added a `changelog.md` entry and rebuilt the package sources according to [this short guide](https://github.com/expo/expo/blob/main/CONTRIBUTING.md#-before-submitting) - [ ] This diff will work correctly for `npx expo prebuild` & EAS Build (eg: updated a module plugin). - [ ] Conforms with the [Documentation Writing Style Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
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 subscribe to this conversation on GitHub.
Already have an account?
Sign in.
4 participants
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.
See Commits and Changes for more details.
Created by
pull[bot] (v2.0.0-alpha.4)
Can you help keep this open source service alive? 💖 Please sponsor : )