diff --git a/apps/expo-go/android/expoview/src/main/java/host/exp/exponent/ExpoUpdatesAppLoader.kt b/apps/expo-go/android/expoview/src/main/java/host/exp/exponent/ExpoUpdatesAppLoader.kt index baac13d03b8076..f74308fdb17ebb 100644 --- a/apps/expo-go/android/expoview/src/main/java/host/exp/exponent/ExpoUpdatesAppLoader.kt +++ b/apps/expo-go/android/expoview/src/main/java/host/exp/exponent/ExpoUpdatesAppLoader.kt @@ -8,7 +8,7 @@ import expo.modules.core.utilities.EmulatorUtilities import expo.modules.easclient.EASClientID import expo.modules.updates.UpdatesConfiguration import expo.modules.updates.UpdatesUtils -import expo.modules.updates.db.DatabaseHolder +import expo.modules.updates.db.UpdatesDatabase import expo.modules.updates.db.entity.UpdateEntity import expo.modules.updates.launcher.Launcher import expo.modules.updates.loader.FileDownloader @@ -70,7 +70,7 @@ class ExpoUpdatesAppLoader @JvmOverloads constructor( lateinit var exponentSharedPreferences: ExponentSharedPreferences @Inject - lateinit var databaseHolder: DatabaseHolder + lateinit var database: UpdatesDatabase @Inject lateinit var kernel: Kernel @@ -173,7 +173,7 @@ class ExpoUpdatesAppLoader @JvmOverloads constructor( EASClientID(context).uuid.toString(), configuration, logger, - databaseHolder.database + database ) loaderScope.launch { startLoaderTask(configuration, fileDownloader, directory, selectionPolicy, context, logger) @@ -192,7 +192,7 @@ class ExpoUpdatesAppLoader @JvmOverloads constructor( LoaderTask( context, configuration, - databaseHolder, + database, directory, fileDownloader, selectionPolicy, diff --git a/apps/expo-go/android/expoview/src/main/java/host/exp/exponent/di/NativeModuleDepsProvider.kt b/apps/expo-go/android/expoview/src/main/java/host/exp/exponent/di/NativeModuleDepsProvider.kt index 63ef616ebd9e55..06c7b70975d269 100644 --- a/apps/expo-go/android/expoview/src/main/java/host/exp/exponent/di/NativeModuleDepsProvider.kt +++ b/apps/expo-go/android/expoview/src/main/java/host/exp/exponent/di/NativeModuleDepsProvider.kt @@ -6,7 +6,6 @@ import android.content.Context import android.os.Handler import android.os.Looper import com.facebook.proguard.annotations.DoNotStrip -import expo.modules.updates.db.DatabaseHolder import expo.modules.updates.db.UpdatesDatabase import host.exp.exponent.ExpoHandler import host.exp.exponent.ExponentManifest @@ -54,7 +53,7 @@ class NativeModuleDepsProvider(application: Application) { @Inject @DoNotStrip - val mUpdatesDatabaseHolder: DatabaseHolder = DatabaseHolder(UpdatesDatabase.getInstance(mContext, Dispatchers.IO)) + val mUpdatesDatabase: UpdatesDatabase = UpdatesDatabase.getInstance(mContext, Dispatchers.IO) private val classToInstanceMap = mutableMapOf, Any>() diff --git a/apps/test-suite/TestModules.ts b/apps/test-suite/TestModules.ts index 4756378035e562..3cb1fb11e93b5a 100644 --- a/apps/test-suite/TestModules.ts +++ b/apps/test-suite/TestModules.ts @@ -86,6 +86,7 @@ export function getTestModules() { if (['android', 'ios'].includes(Platform.OS)) { modules.push(require('./tests/ExpoUIMeasurement')); modules.push(require('./tests/ExpoUIHostSize')); + modules.push(require('./tests/ExpoUIRNHostViewSize')); modules.push(require('./tests/AppMetrics')); modules.push(require('./tests/Blob')); modules.push(require('./tests/FileSystem')); diff --git a/apps/test-suite/components/Suites.tsx b/apps/test-suite/components/Suites.tsx index 72ab8537b3d986..a077eb26366740 100644 --- a/apps/test-suite/components/Suites.tsx +++ b/apps/test-suite/components/Suites.tsx @@ -299,8 +299,8 @@ export default function Suites({ renderItem={({ item }) => ( )} - ListHeaderComponent={header} - ListFooterComponent={footer} + ListHeaderComponent={header ?? undefined} + ListFooterComponent={footer ?? undefined} stickyHeaderIndices={[0]} onContentSizeChange={onContentSizeChange} onLayout={onLayout} diff --git a/apps/test-suite/tests/ExpoUIMeasurement.android.tsx b/apps/test-suite/tests/ExpoUIMeasurement.android.tsx index 5e949cb4b39afc..1235dd4b77f1d1 100644 --- a/apps/test-suite/tests/ExpoUIMeasurement.android.tsx +++ b/apps/test-suite/tests/ExpoUIMeasurement.android.tsx @@ -4,6 +4,10 @@ import { padding, paddingAll, size } from '@expo/ui/jetpack-compose/modifiers'; import React from 'react'; import { ScrollView, View } from 'react-native'; +// React Native types `View` as a function component, so the instance type is its ref type. +type ViewRef = React.ComponentRef; +type ScrollViewRef = React.ComponentRef; + // Tests actual placement of UI matches return returned by RN's measure API. // UI is placed by Compose and not Yoga, so these tests test that correct placement is reported to RN so Pressability can work correctly. export const name = 'ExpoUIMeasurement'; @@ -29,7 +33,7 @@ type Measurement = { pageY: number; }; -function measureAsync(ref: React.RefObject, label = 'view'): Promise { +function measureAsync(ref: React.RefObject, label = 'view'): Promise { return new Promise((resolve, reject) => { const node = ref.current; if (!node) { @@ -51,7 +55,7 @@ function delay(ms: number): Promise { * a row. * */ -async function measureWhenSettled>>( +async function measureWhenSettled>>( refs: T, timeoutMs = 5000 ): Promise> { @@ -109,8 +113,8 @@ export async function test( describe(name, () => { it('measures a hosted view where Compose placed it', async () => { - const hostWrapperRef = React.createRef(); - const hostedRef = React.createRef(); + const hostWrapperRef = React.createRef(); + const hostedRef = React.createRef(); setPortalChild( @@ -137,8 +141,8 @@ export async function test( }); it('measures a hosted view offset by its own padding modifier', async () => { - const hostWrapperRef = React.createRef(); - const hostedRef = React.createRef(); + const hostWrapperRef = React.createRef(); + const hostedRef = React.createRef(); // The padding sits on the `RNHostView` itself, which is what the universal `RNHostView` // makes of `style={{ padding }}`. Compose applies a modifier chain outside-in, so the @@ -163,8 +167,8 @@ export async function test( }); it('measures a hosted view offset by a padded Compose column', async () => { - const hostWrapperRef = React.createRef(); - const hostedRef = React.createRef(); + const hostWrapperRef = React.createRef(); + const hostedRef = React.createRef(); setPortalChild( @@ -188,9 +192,9 @@ export async function test( }); it('measures a hosted view stacked below another hosted view', async () => { - const hostWrapperRef = React.createRef(); - const firstRef = React.createRef(); - const secondRef = React.createRef(); + const hostWrapperRef = React.createRef(); + const firstRef = React.createRef(); + const secondRef = React.createRef(); setPortalChild( @@ -222,8 +226,8 @@ export async function test( }); it('measures a hosted view in a row, beside a Compose-only sibling', async () => { - const hostWrapperRef = React.createRef(); - const hostedRef = React.createRef(); + const hostWrapperRef = React.createRef(); + const hostedRef = React.createRef(); setPortalChild( @@ -248,9 +252,9 @@ export async function test( }); it('measures a hosted view in a row, beside another hosted view', async () => { - const hostWrapperRef = React.createRef(); - const firstRef = React.createRef(); - const secondRef = React.createRef(); + const hostWrapperRef = React.createRef(); + const firstRef = React.createRef(); + const secondRef = React.createRef(); setPortalChild( @@ -280,12 +284,12 @@ export async function test( }); it('measures a hosted view when the Host is inside a React Native ScrollView', async () => { - const scrollRef = React.createRef(); + const scrollRef = React.createRef(); // Anchored outside the `ScrollView` so it does not move with the content. A difference between // two views that scroll together would cancel the scroll offset out and never notice it. - const viewportRef = React.createRef(); - const hostWrapperRef = React.createRef(); - const hostedRef = React.createRef(); + const viewportRef = React.createRef(); + const hostWrapperRef = React.createRef(); + const hostedRef = React.createRef(); setPortalChild( @@ -332,10 +336,10 @@ export async function test( }); it('measures a PagerView page where Compose drew it, after paging to it', async () => { - const pagerWrapperRef = React.createRef(); + const pagerWrapperRef = React.createRef(); const pagerRef = React.createRef(); - const firstRef = React.createRef(); - const secondRef = React.createRef(); + const firstRef = React.createRef(); + const secondRef = React.createRef(); let onSelected: ((position: number) => void) | null = null; const selected = new Promise((resolve) => { @@ -371,7 +375,7 @@ export async function test( }); it('measures a hosted view in a modal bottom sheet relative to itself', async () => { - const hostedRef = React.createRef(); + const hostedRef = React.createRef(); setPortalChild( diff --git a/apps/test-suite/tests/ExpoUIMeasurement.ios.tsx b/apps/test-suite/tests/ExpoUIMeasurement.ios.tsx index 8118f8546a9b68..14b34e69e826d6 100644 --- a/apps/test-suite/tests/ExpoUIMeasurement.ios.tsx +++ b/apps/test-suite/tests/ExpoUIMeasurement.ios.tsx @@ -3,6 +3,10 @@ import { padding } from '@expo/ui/swift-ui/modifiers'; import React from 'react'; import { ScrollView, View } from 'react-native'; +// React Native types `View` as a function component, so the instance type is its ref type. +type ViewRef = React.ComponentRef; +type ScrollViewRef = React.ComponentRef; + // Tests that the actual placement of the UI matches what RN's measure API returns. // UI is placed by SwiftUI and not Yoga, so these tests check that the placement reported to RN is correct and Pressability works. export const name = 'ExpoUIMeasurement'; @@ -23,7 +27,7 @@ type Measurement = { pageY: number; }; -function measureAsync(ref: React.RefObject, label = 'view'): Promise { +function measureAsync(ref: React.RefObject, label = 'view'): Promise { return new Promise((resolve, reject) => { const node = ref.current; if (!node) { @@ -45,7 +49,7 @@ function delay(ms: number): Promise { * mounts when it is on screen and animates in, so there is no single layout callback to wait on. */ async function measureWhenPresented( - ref: React.RefObject, + ref: React.RefObject, label: string, timeoutMs = 5000 ): Promise { @@ -72,8 +76,8 @@ export async function test( describe(name, () => { it('measures a hosted view where SwiftUI placed it, not where Yoga put its box', async () => { - const hostWrapperRef = React.createRef(); - const hostedRef = React.createRef(); + const hostWrapperRef = React.createRef(); + const hostedRef = React.createRef(); let onLaidOut: () => void; const laidOut = new Promise((resolve) => { @@ -105,9 +109,9 @@ export async function test( }); it('measures a hosted view stacked below another hosted view', async () => { - const hostWrapperRef = React.createRef(); - const firstRef = React.createRef(); - const secondRef = React.createRef(); + const hostWrapperRef = React.createRef(); + const firstRef = React.createRef(); + const secondRef = React.createRef(); let onLaidOut: () => void; const laidOut = new Promise((resolve) => { @@ -142,8 +146,8 @@ export async function test( }); it('measures a hosted view in a row, beside a SwiftUI-only sibling', async () => { - const hostWrapperRef = React.createRef(); - const hostedRef = React.createRef(); + const hostWrapperRef = React.createRef(); + const hostedRef = React.createRef(); let onLaidOut: () => void; const laidOut = new Promise((resolve) => { @@ -173,9 +177,9 @@ export async function test( }); it('measures a hosted view in a row, beside another hosted view', async () => { - const hostWrapperRef = React.createRef(); - const firstRef = React.createRef(); - const secondRef = React.createRef(); + const hostWrapperRef = React.createRef(); + const firstRef = React.createRef(); + const secondRef = React.createRef(); let onLaidOut: () => void; const laidOut = new Promise((resolve) => { @@ -210,14 +214,14 @@ export async function test( }); it('measures a hosted view when the Host is inside a React Native ScrollView', async () => { - const scrollRef = React.createRef(); + const scrollRef = React.createRef(); // Anchored outside the `ScrollView`, so it does not move when the content does. Measuring // against it gives the box's real position on screen, which is what a touch is compared // against — a difference between two views that scroll together would cancel the scroll // offset out and never notice if it were wrong. - const viewportRef = React.createRef(); - const hostWrapperRef = React.createRef(); - const hostedRef = React.createRef(); + const viewportRef = React.createRef(); + const hostWrapperRef = React.createRef(); + const hostedRef = React.createRef(); let onLaidOut: () => void; const laidOut = new Promise((resolve) => { @@ -296,7 +300,7 @@ export async function test( // A sheet content uses RootNodeKind trait so measurement happens relative to the RNHostView and not the RN's root surface. it('measures a hosted view in a sheet relative to itself', async () => { - const hostedRef = React.createRef(); + const hostedRef = React.createRef(); setPortalChild( @@ -324,7 +328,7 @@ export async function test( }); it('measures a hosted view nested inside sheet content from the outer hosted view', async () => { - const nestedRef = React.createRef(); + const nestedRef = React.createRef(); setPortalChild( diff --git a/apps/test-suite/tests/ExpoUIRNHostViewSize.tsx b/apps/test-suite/tests/ExpoUIRNHostViewSize.tsx new file mode 100644 index 00000000000000..34f28dcee2d298 --- /dev/null +++ b/apps/test-suite/tests/ExpoUIRNHostViewSize.tsx @@ -0,0 +1,365 @@ +import { Column, Host, RNHostView } from '@expo/ui'; +import React from 'react'; +import { Text, View, type LayoutChangeEvent } from 'react-native'; + +import type { JasmineInterface, TestPortal } from '../types'; +import { mountAndWaitForWithTimeout } from './helpers'; + +export const name = 'ExpoUI RNHostView size'; +export const route = 'expo-ui-rnhostview-size'; + +const SETTLE_MS = 250; +const TIMEOUT_MS = 10000; +const TOLERANCE = 1; + +const PARENT_WIDTH = 300; +const BOX_WIDTH = 40; +const BOX_HEIGHT = 20; +const HOST_WIDTH = 200; +const HOST_HEIGHT = 120; +const COLUMN_PADDING = 8; +const TEXT = 'Hello, world!'; +const LONG_TEXT = 'a much longer string that has to wrap when the width is limited'; +const FONT_SIZE = 24; +const CONTENT_MAX_WIDTH = 140; +const CONTENT_MIN_HEIGHT = 60; +const LOOP_WINDOW_MS = 1500; + +type Axes = { width: number; height: number }; +type Paired = { hosted: Axes; control: Axes }; +type AcrossChange = { before: Axes; grown: Axes; shrunk: Axes }; + +function useSettledLayout(onSettled: (size: Axes) => void) { + const size = React.useRef(undefined); + const settle = React.useRef | undefined>(undefined); + const onSettledRef = React.useRef(onSettled); + + React.useEffect(() => { + onSettledRef.current = onSettled; + }); + + React.useEffect(() => () => clearTimeout(settle.current), []); + + return React.useCallback((event: { nativeEvent: { layout: Axes } }) => { + const { width, height } = event.nativeEvent.layout; + size.current = { width, height }; + + clearTimeout(settle.current); + settle.current = setTimeout(() => { + if (size.current) { + onSettledRef.current(size.current); + } + }, SETTLE_MS); + }, []); +} + +function HuggingProbe({ onMeasured }: { onMeasured: (size: Axes) => void }) { + return ( + + + + onMeasured(nativeEvent.layout)}> + + + + + + + + + ); +} + +function FillingProbe({ onMeasured }: { onMeasured: (size: Axes) => void }) { + const onLayout = useSettledLayout(onMeasured); + + return ( + + + + + + + + + + + ); +} + +function CrossAxisProbe({ onMeasured }: { onMeasured: (size: Axes) => void }) { + return ( + + + onMeasured(nativeEvent.layout)}> + + + + + ); +} + +// Regression test for Host + RNHostView layout loops https://github.com/expo/expo/pull/48059#issuecomment-5351404485 +function PaddedHostProbe({ onMeasured }: { onMeasured: (sizes: Paired) => void }) { + const sizes = React.useRef>({}); + + const report = (key: keyof Paired, layout: Axes) => { + sizes.current[key] = { width: layout.width, height: layout.height }; + + const { hosted, control } = sizes.current; + if (hosted && control) { + onMeasured({ hosted, control }); + } + }; + + return ( + + + + report('hosted', nativeEvent.layout)}> + {TEXT} + + + + report('control', nativeEvent.layout)}> + {TEXT} + + + ); +} + +function ContentLimitsProbe({ onMeasured }: { onMeasured: (size: Axes) => void }) { + const onLayout = useSettledLayout(onMeasured); + + return ( + + + + + {LONG_TEXT} + + + + + ); +} + +function HostedOnLayoutProbe({ onMeasured }: { onMeasured: (sizes: Paired) => void }) { + const sizes = React.useRef>({}); + + const report = (key: keyof Paired, layout: Axes) => { + sizes.current[key] = { width: layout.width, height: layout.height }; + + const { hosted, control } = sizes.current; + if (hosted && control) { + onMeasured({ hosted, control }); + } + }; + + return ( + + + report('control', nativeEvent.layout)}> + report('hosted', nativeEvent.layout)} + /> + + + + ); +} + +function ResizeProbe({ onMeasured }: { onMeasured: (sizes: AcrossChange) => void }) { + const [step, setStep] = React.useState(0); + const seen = React.useRef>({}); + + const onLayout = useSettledLayout((size) => { + if (step === 0) { + seen.current.before = size; + setStep(1); + } else if (step === 1) { + seen.current.grown = size; + setStep(2); + } else if (step === 2) { + const { before, grown } = seen.current; + if (before && grown) { + onMeasured({ before, grown, shrunk: size }); + } + } + }); + + return ( + + + + + {step === 1 ? LONG_TEXT : TEXT} + + + + + ); +} + +// Regression test for the layout loop in https://github.com/expo/expo/issues/48058. +// It needs a constant amount of native chrome around content that stretches to the width it is +// offered. Measuring that content at the width the previous pass produced added the chrome again +// every round, so the width grew by a constant forever. +function DivergenceProbe({ onMeasured }: { onMeasured: (widths: number[]) => void }) { + const widths = React.useRef([]); + const closed = React.useRef(false); + const timer = React.useRef | undefined>(undefined); + + React.useEffect(() => () => clearTimeout(timer.current), []); + + const onLayout = ({ nativeEvent }: LayoutChangeEvent) => { + if (closed.current) { + return; + } + widths.current.push(nativeEvent.layout.width); + + timer.current ??= setTimeout(() => { + closed.current = true; + onMeasured(widths.current); + }, LOOP_WINDOW_MS); + }; + + return ( + + + + + + + + + + + + + ); +} + +export async function test( + { it, describe, expect, afterEach }: JasmineInterface, + { setPortalChild, cleanupPortal }: TestPortal +) { + describe(name, () => { + afterEach(async () => { + await cleanupPortal(); + }); + + it('lays out hosted content at its own size, not at the width the parent offers', async () => { + const size = await mountAndWaitForWithTimeout( + {}} />, + 'onMeasured', + setPortalChild, + TIMEOUT_MS + ); + + expect(Math.abs(size.width - BOX_WIDTH)).toBeLessThan(TOLERANCE); + expect(Math.abs(size.height - BOX_HEIGHT)).toBeLessThan(TOLERANCE); + }); + + it('spreads hosted content across its native parent without matchContents', async () => { + const size = await mountAndWaitForWithTimeout( + {}} />, + 'onMeasured', + setPortalChild, + TIMEOUT_MS + ); + + expect(Math.abs(size.width - HOST_WIDTH)).toBeLessThan(TOLERANCE); + expect(Math.abs(size.height - HOST_HEIGHT)).toBeLessThan(TOLERANCE); + }); + + it('hugs fixed-size content on the cross axis instead of filling the host', async () => { + const size = await mountAndWaitForWithTimeout( + {}} />, + 'onMeasured', + setPortalChild, + TIMEOUT_MS + ); + + expect(Math.abs(size.width - BOX_WIDTH)).toBeLessThan(TOLERANCE); + expect(Math.abs(size.height - BOX_HEIGHT)).toBeLessThan(TOLERANCE); + }); + + it('hugs hosted text inside a natively padded host', async () => { + const { hosted, control } = await mountAndWaitForWithTimeout( + {}} />, + 'onMeasured', + setPortalChild, + TIMEOUT_MS + ); + + expect(control.width).toBeGreaterThan(0); + expect(Math.abs(hosted.width - control.width)).toBeLessThan(TOLERANCE); + expect(Math.abs(hosted.height - control.height)).toBeLessThan(TOLERANCE); + expect(hosted.width).toBeLessThan(PARENT_WIDTH); + }); + + it("honours the hosted view's own maxWidth and minHeight", async () => { + const size = await mountAndWaitForWithTimeout( + {}} />, + 'onMeasured', + setPortalChild, + TIMEOUT_MS + ); + + expect(Math.abs(size.width - CONTENT_MAX_WIDTH)).toBeLessThan(TOLERANCE); + expect(size.height).toBeGreaterThan(FONT_SIZE * 1.5); + expect(size.height).toBeGreaterThanOrEqual(CONTENT_MIN_HEIGHT - TOLERANCE); + }); + + it('fires onLayout on the hosted element, at the size the host reports', async () => { + const { hosted, control } = await mountAndWaitForWithTimeout( + {}} />, + 'onMeasured', + setPortalChild, + TIMEOUT_MS + ); + + expect(Math.abs(hosted.width - BOX_WIDTH)).toBeLessThan(TOLERANCE); + expect(Math.abs(hosted.height - BOX_HEIGHT)).toBeLessThan(TOLERANCE); + expect(Math.abs(hosted.width - control.width)).toBeLessThan(TOLERANCE); + expect(Math.abs(hosted.height - control.height)).toBeLessThan(TOLERANCE); + }); + + // Regression test for https://github.com/expo/expo/issues/47883 + it('tracks hosted content that changes size after mount, both ways', async () => { + const { before, grown, shrunk } = await mountAndWaitForWithTimeout( + {}} />, + 'onMeasured', + setPortalChild, + TIMEOUT_MS + ); + + expect(grown.width).toBeGreaterThan(before.width); + expect(Math.abs(shrunk.width - before.width)).toBeLessThan(TOLERANCE); + }); + + // Regression test for https://github.com/expo/expo/issues/48058 + it('settles at the content width when native chrome wraps stretching content', async () => { + const widths = await mountAndWaitForWithTimeout( + {}} />, + 'onMeasured', + setPortalChild, + TIMEOUT_MS + ); + + expect(widths.length).toBeGreaterThan(0); + // A diverging layout grew the width for as long as it was measured. + expect(Math.abs(widths[widths.length - 1] - widths[0])).toBeLessThan(TOLERANCE); + // The width the parent offers never reaches the content. + expect(Math.abs(Math.max(...widths) - BOX_WIDTH)).toBeLessThan(TOLERANCE); + }); + }); +} diff --git a/docs/pages/eas/workflows/syntax.mdx b/docs/pages/eas/workflows/syntax.mdx index 56e18ce05f3fe6..027f0b9a2187e9 100644 --- a/docs/pages/eas/workflows/syntax.mdx +++ b/docs/pages/eas/workflows/syntax.mdx @@ -136,6 +136,8 @@ on: Runs your workflow when you create or update a pull request that targets one of the matching branches. +> **info** Pull requests opened from a fork of the connected repository do not trigger `pull_request` workflow runs. + With the `branches` list, you can trigger the workflow only when those specified branches are the target of the pull request. For example, if you use `branches: ['main']`, only pull requests to merge into the main branch trigger the workflow. Supports globs. Defaults to `['*']` when not provided, which means the workflow triggers on pull request events to all branches. By using the `!` prefix you can specify branches to ignore (you still need to provide at least one branch pattern without it). With the `types` list, you can trigger the workflow only on the specified pull request event types. For example, if you use `types: ['opened']`, only the `pull_request.opened` event (sent when a pull request is first opened) triggers the workflow. Defaults to `['opened', 'reopened', 'synchronize']` when not provided. Supported event types: diff --git a/docs/pages/versions/unversioned/sdk/ui/jetpack-compose/rnhostview.mdx b/docs/pages/versions/unversioned/sdk/ui/jetpack-compose/rnhostview.mdx index 4984f9bf9ec1f2..bd9503beb0c342 100644 --- a/docs/pages/versions/unversioned/sdk/ui/jetpack-compose/rnhostview.mdx +++ b/docs/pages/versions/unversioned/sdk/ui/jetpack-compose/rnhostview.mdx @@ -17,6 +17,8 @@ When React Native views are placed inside Jetpack Compose components like [`Moda - **With [`matchContents`](#matchcontents)**: The shadow node size is set to match the child React Native view's intrinsic size, allowing the Jetpack Compose parent to size itself based on the React Native content. - **Without `matchContents`**: The shadow node size is set to match the parent Jetpack Compose view's size, allowing the React Native content to fill the available space (useful for `flex: 1` layouts). +> **Note:** Pass a single React Native element as the child. `RNHostView` measures and lays out only its first child, so wrap several views in one parent `View` and let that view arrange them. + ## Installation diff --git a/docs/pages/versions/unversioned/sdk/ui/swift-ui/rnhostview.mdx b/docs/pages/versions/unversioned/sdk/ui/swift-ui/rnhostview.mdx index 1397562774e5ea..f17eca3a22c997 100644 --- a/docs/pages/versions/unversioned/sdk/ui/swift-ui/rnhostview.mdx +++ b/docs/pages/versions/unversioned/sdk/ui/swift-ui/rnhostview.mdx @@ -16,6 +16,8 @@ When React Native views are placed inside SwiftUI components like [`BottomSheet` - **With `matchContents`**: The shadow node size is set to match the child React Native view's intrinsic size, allowing the SwiftUI parent to size itself based on the React Native content. - **Without `matchContents`**: The shadow node size is set to match the parent SwiftUI view's size, allowing the React Native content to fill the available space (useful for `flex: 1` layouts). +> **Note:** Pass a single React Native element as the child. `RNHostView` measures and lays out only its first child, so wrap several views in one parent `View` and let that view arrange them. + ## Installation diff --git a/docs/pages/versions/unversioned/sdk/ui/universal/rnhostview.mdx b/docs/pages/versions/unversioned/sdk/ui/universal/rnhostview.mdx index a25c73b1c2fbf4..b0e80bd7da5244 100644 --- a/docs/pages/versions/unversioned/sdk/ui/universal/rnhostview.mdx +++ b/docs/pages/versions/unversioned/sdk/ui/universal/rnhostview.mdx @@ -13,6 +13,8 @@ import { PlatformSpotlight, PlatformTabsGroup } from '~/ui/components/PlatformTa Hosts a React Native view subtree inside a universal `@expo/ui` layout. On Android and iOS, it re-exports the platform-native [`RNHostView` for Jetpack Compose](../jetpack-compose/rnhostview)/[`RNHostView` for SwiftUI](../swift-ui/rnhostview), so React Native children bridge into the surrounding Compose/SwiftUI tree. On web, there is no native host tree to bridge into, so it falls back to a React Native [`View`](https://reactnative.dev/docs/view) that wraps the children. +> **Note:** Pass a single React Native element as the child. `RNHostView` measures and lays out only its first child, so wrap several views in one parent `View` and let that view arrange them. + **Note:** Pass a single React Native element as the child. `RNHostView` measures and lays out only its first child, so wrap several views in one parent `View` and let that view arrange them. + ## Installation diff --git a/docs/pages/versions/v57.0.0/sdk/ui/swift-ui/rnhostview.mdx b/docs/pages/versions/v57.0.0/sdk/ui/swift-ui/rnhostview.mdx index e7a603c822adf4..aafc78a15aad46 100644 --- a/docs/pages/versions/v57.0.0/sdk/ui/swift-ui/rnhostview.mdx +++ b/docs/pages/versions/v57.0.0/sdk/ui/swift-ui/rnhostview.mdx @@ -16,6 +16,8 @@ When React Native views are placed inside SwiftUI components like [`BottomSheet` - **With `matchContents`**: The shadow node size is set to match the child React Native view's intrinsic size, allowing the SwiftUI parent to size itself based on the React Native content. - **Without `matchContents`**: The shadow node size is set to match the parent SwiftUI view's size, allowing the React Native content to fill the available space (useful for `flex: 1` layouts). +> **Note:** Pass a single React Native element as the child. `RNHostView` measures and lays out only its first child, so wrap several views in one parent `View` and let that view arrange them. + ## Installation diff --git a/docs/pages/versions/v57.0.0/sdk/ui/universal/rnhostview.mdx b/docs/pages/versions/v57.0.0/sdk/ui/universal/rnhostview.mdx index 072a47d68c6055..2718ef35535e02 100644 --- a/docs/pages/versions/v57.0.0/sdk/ui/universal/rnhostview.mdx +++ b/docs/pages/versions/v57.0.0/sdk/ui/universal/rnhostview.mdx @@ -13,6 +13,8 @@ import { PlatformSpotlight, PlatformTabsGroup } from '~/ui/components/PlatformTa Hosts a React Native view subtree inside a universal `@expo/ui` layout. On Android and iOS, it re-exports the platform-native [`RNHostView` for Jetpack Compose](../jetpack-compose/rnhostview)/[`RNHostView` for SwiftUI](../swift-ui/rnhostview), so React Native children bridge into the surrounding Compose/SwiftUI tree. On web, there is no native host tree to bridge into, so it falls back to a React Native [`View`](https://reactnative.dev/docs/view) that wraps the children. +> **Note:** Pass a single React Native element as the child. `RNHostView` measures and lays out only its first child, so wrap several views in one parent `View` and let that view arrange them. + 0 && markdownQuality >= htmlQuality; +} + function upgradeHelperPairPath(url) { if (!/^\/bare\/upgrade\/?$/.test(url.pathname)) return null; @@ -18,7 +56,7 @@ export default { const pairPath = upgradeHelperPairPath(url); const wantsMarkdown = - accept.includes("text/markdown") || + acceptsMarkdown(accept) || (pairPath !== null && /\.md$/.test(url.searchParams.get("toSdk") || "")); if (wantsMarkdown) { @@ -40,14 +78,29 @@ export default { status: 200, headers: { "Content-Type": "text/markdown; charset=utf-8", + Vary: "Accept", }, }); } } - return new Response("Not found\n", { status: 404 }); + const passthrough = await env.ASSETS.fetch(request); + if (passthrough.status >= 300 && passthrough.status < 400) { + return passthrough; + } + + return new Response(NOT_FOUND_MARKDOWN, { + status: 404, + headers: { + "Content-Type": "text/markdown; charset=utf-8", + Vary: "Accept", + }, + }); } - return env.ASSETS.fetch(request); + const htmlResponse = await env.ASSETS.fetch(request); + const response = new Response(htmlResponse.body, htmlResponse); + response.headers.append("Vary", "Accept"); + return response; }, }; diff --git a/docs/scripts/test-worker.ts b/docs/scripts/test-worker.ts index 440fb23f0b461e..e113765590aca8 100644 --- a/docs/scripts/test-worker.ts +++ b/docs/scripts/test-worker.ts @@ -144,6 +144,13 @@ async function testMarkdownContentNegotiationAsync(): Promise { } console.log('✓ Normal request serves HTML'); + if (!varyTokens(htmlResponse).includes('accept')) { + throw new Error( + `Expected Vary header listing Accept on the HTML variant, got: ${htmlResponse.headers.get('vary') ?? '(absent)'}` + ); + } + console.log('✓ HTML variant of a negotiated page returns Vary: Accept'); + // With Accept: text/markdown, should serve markdown const mdResponse = await fetch(`${BASE_URL}/test-page`, { headers: { Accept: 'text/markdown' }, @@ -161,6 +168,17 @@ async function testMarkdownContentNegotiationAsync(): Promise { throw new Error(`Expected Content-Type text/markdown, got: ${mdContentType}`); } console.log('✓ Accept: text/markdown request returns correct Content-Type'); + + if (!varyTokens(mdResponse).includes('accept')) { + throw new Error( + `Expected Vary header listing Accept, got: ${mdResponse.headers.get('vary') ?? '(absent)'}` + ); + } + console.log('✓ Accept: text/markdown request returns Vary: Accept'); +} + +function varyTokens(response: Response): string[] { + return (response.headers.get('vary') ?? '').split(',').map(token => token.trim().toLowerCase()); } async function testMarkdownNotFoundAsync(): Promise { @@ -176,6 +194,28 @@ async function testMarkdownNotFoundAsync(): Promise { } console.log('✓ Missing .md file returns 404'); + if (!varyTokens(missingMd).includes('accept')) { + throw new Error( + `Expected Vary header listing Accept on the markdown 404, got: ${missingMd.headers.get('vary') ?? '(absent)'}` + ); + } + console.log('✓ Markdown 404 response includes Vary: Accept'); + + const missingMdType = missingMd.headers.get('content-type') ?? ''; + + if (!missingMdType.includes('text/markdown')) { + throw new Error(`Expected markdown Content-Type on the 404, got: ${missingMdType}`); + } + + const missingMdBody = await missingMd.text(); + + if (!missingMdBody.includes('/llms.txt') || !missingMdBody.includes('/sitemap.xml')) { + throw new Error( + `Expected recovery links on the markdown 404, got: ${missingMdBody.slice(0, 200)}` + ); + } + console.log('✓ Markdown 404 responds with a markdown body and recovery links'); + // Nonexistent page should also 404 const notFound = await fetch(`${BASE_URL}/nonexistent-page`, { headers: { Accept: 'text/markdown' }, @@ -187,6 +227,46 @@ async function testMarkdownNotFoundAsync(): Promise { console.log('✓ Nonexistent page returns 404'); } +async function testAcceptQualityValuesAsync(): Promise { + console.log('\n--- Testing Accept header q-values ---'); + + const htmlPreferred = await fetch(`${BASE_URL}/test-page`, { + headers: { Accept: 'text/html;q=0.9, text/markdown;q=0.1' }, + }); + const htmlPreferredBody = await htmlPreferred.text(); + + if (!htmlPreferredBody.includes('Test Page HTML')) { + throw new Error( + 'Expected HTML when it carries the higher q, got: ' + htmlPreferredBody.slice(0, 200) + ); + } + console.log('✓ HTML with the higher q is served over markdown'); + + const markdownRejected = await fetch(`${BASE_URL}/test-page`, { + headers: { Accept: 'text/markdown;q=0, text/html' }, + }); + const markdownRejectedBody = await markdownRejected.text(); + + if (!markdownRejectedBody.includes('Test Page HTML')) { + throw new Error( + 'Expected HTML when markdown has q=0, got: ' + markdownRejectedBody.slice(0, 200) + ); + } + console.log('✓ Markdown with q=0 is never served'); + + const markdownPreferred = await fetch(`${BASE_URL}/test-page`, { + headers: { Accept: 'text/markdown;q=0.9, text/html;q=0.1' }, + }); + const markdownPreferredBody = await markdownPreferred.text(); + + if (!markdownPreferredBody.includes('Test Markdown Content')) { + throw new Error( + 'Expected markdown when it carries the higher q, got: ' + markdownPreferredBody.slice(0, 200) + ); + } + console.log('✓ Markdown with the higher q is served'); +} + async function testUpgradePairNegotiationAsync(): Promise { console.log('\n--- Testing upgrade helper version-pair negotiation ---'); @@ -328,6 +408,19 @@ async function testDeletedPageRedirectsAsync(): Promise { ); } console.log('✓ expo-go-to-dev-build redirects to the introduction build locally section'); + + const mdRedirect = await fetch(`${BASE_URL}/develop/development-builds/create-a-build`, { + headers: { Accept: 'text/markdown' }, + redirect: 'manual', + }); + const mdLocation = mdRedirect.headers.get('location') ?? ''; + + if (mdRedirect.status !== 301 || !mdLocation.includes('?buildenv=build-with-eas')) { + throw new Error( + `Expected the markdown request to pass through the page redirect, got: HTTP ${mdRedirect.status} -> ${mdLocation}` + ); + } + console.log('✓ Accept: text/markdown request passes through page redirects'); } async function testAgentDiscoveryRedirectsAsync(): Promise { @@ -379,6 +472,7 @@ async function mainAsync(): Promise { await testDirectMarkdownAccessAsync(); await testMarkdownContentNegotiationAsync(); await testMarkdownNotFoundAsync(); + await testAcceptQualityValuesAsync(); await testUpgradePairNegotiationAsync(); await testDeletedPageRedirectsAsync(); await testAgentDiscoveryRedirectsAsync(); diff --git a/docs/ui/components/Authentication/Grid.tsx b/docs/ui/components/Authentication/Grid.tsx deleted file mode 100644 index 9fc532d7daf202..00000000000000 --- a/docs/ui/components/Authentication/Grid.tsx +++ /dev/null @@ -1,7 +0,0 @@ -import { type PropsWithChildren } from 'react'; - -export const Grid = ({ children }: PropsWithChildren) => ( -
- {children} -
-); diff --git a/docs/ui/components/Authentication/GridItem.tsx b/docs/ui/components/Authentication/GridItem.tsx deleted file mode 100644 index 5aa4453d90c96a..00000000000000 --- a/docs/ui/components/Authentication/GridItem.tsx +++ /dev/null @@ -1,40 +0,0 @@ -import { mergeClasses } from '@expo/styleguide'; - -import { A, CALLOUT, RawH4 } from '~/ui/components/Text'; - -import { Icon } from './Icon'; - -type GridItemProps = React.PropsWithChildren<{ - title: string; - image?: string; - href?: string; - protocol: string[]; -}>; - -export const GridItem = ({ - title, - image, - protocol = [], - href = `#${title.toLowerCase().replaceAll(' ', '-')}`, -}: GridItemProps) => ( - - - {title} - {(protocol || []).length > 0 && ( - - {protocol.join(' | ')} - - )} - -); diff --git a/docs/ui/components/Authentication/index.ts b/docs/ui/components/Authentication/index.ts index ea7c99ac0d0481..d26f2e45e456f2 100644 --- a/docs/ui/components/Authentication/index.ts +++ b/docs/ui/components/Authentication/index.ts @@ -1,6 +1,4 @@ export { Box } from './Box'; -export { Grid } from './Grid'; -export { GridItem } from './GridItem'; const ASSETS_PATH = '/static/images/sdk/auth-session/'; export const ASSETS = { diff --git a/package.json b/package.json index a0e7f731312065..1b600044a04c6c 100644 --- a/package.json +++ b/package.json @@ -41,7 +41,7 @@ "jest": "^29.7.0", "lint-staged": "^17.0.8", "oxfmt": "^0.55.0", - "oxlint": "^1.70.0", + "oxlint": "^1.79.0", "prettier": "~3.8.3", "ts-node": "^10.9.2", "turbo": "2.10.0", diff --git a/packages/@expo/cli/e2e/jest/test-windows.ts b/packages/@expo/cli/e2e/jest/test-windows.ts index 1cf73fe41d4219..fadceec66ca4b7 100644 --- a/packages/@expo/cli/e2e/jest/test-windows.ts +++ b/packages/@expo/cli/e2e/jest/test-windows.ts @@ -10,5 +10,3 @@ declare global { /** Run a test case on any platform except Windows */ var testNotWindows: jest.It; } - -export {}; diff --git a/packages/@expo/log-box/src/logbox-dom-polyfill.tsx b/packages/@expo/log-box/src/logbox-dom-polyfill.tsx index c5d3c0bbdf195b..d892572510a594 100644 --- a/packages/@expo/log-box/src/logbox-dom-polyfill.tsx +++ b/packages/@expo/log-box/src/logbox-dom-polyfill.tsx @@ -93,13 +93,11 @@ function useNativeLogBoxDataPolyfill( } ) { // @ts-ignore - // eslint-disable-next-line import/namespace - // oxlint-disable-next-line no-import-assign + // oxlint-disable-next-line no-import-assign, import/namespace LogBoxData.setSelectedLog = polyfill.onChangeSelectedIndex; // @ts-ignore - // eslint-disable-next-line import/namespace - // oxlint-disable-next-line no-import-assign + // oxlint-disable-next-line no-import-assign, import/namespace LogBoxData.dismiss = (log: LogBoxLog) => { const index = logs.indexOf(log); polyfill.onDismiss?.(index); diff --git a/packages/expo-module-scripts/CHANGELOG.md b/packages/expo-module-scripts/CHANGELOG.md index 64c579aba46270..1a8cc828829615 100644 --- a/packages/expo-module-scripts/CHANGELOG.md +++ b/packages/expo-module-scripts/CHANGELOG.md @@ -14,6 +14,7 @@ ### 💡 Others - Added `CLAUDE.md` and `CONTRIBUTING.md` to `.npmignore` template. ([#47451](https://github.com/expo/expo/pull/47451) by [@kudo](https://github.com/kudo)) +- Updated `oxlint-config-universe` to 0.2.0, which requires `oxlint` 1.79.0. ([#49241](https://github.com/expo/expo/pull/49241) by [@tsapeta](https://github.com/tsapeta)) ## 56.0.3 - 2026-05-29 diff --git a/packages/expo-module-scripts/oxlint.config.base.js b/packages/expo-module-scripts/oxlint.config.base.js index a8b877f7981f92..4bb3e202133f65 100644 --- a/packages/expo-module-scripts/oxlint.config.base.js +++ b/packages/expo-module-scripts/oxlint.config.base.js @@ -52,16 +52,15 @@ export default defineConfig({ { files: ['**/*.ts', '**/*.tsx', '**/*.d.ts'], rules: { - // Already handled by TypeScript itself, enabling them blocks legitimate type overloads. - 'no-redeclare': 'off', - 'typescript/no-redeclare': 'off', // Handled by TypeScript. 'no-unused-expressions': 'off', - 'typescript/no-unused-expressions': 'off', 'no-unused-vars': 'off', - 'typescript/no-unused-vars': 'off', 'no-useless-return': 'off', + // Enabling it blocks legitimate type overloads, which TypeScript already validates. 'no-dupe-class-members': 'off', + // Function declarations directly inside a TS `namespace` body are legal; the rule's + // default disallows them (the `namespaces` option landed in oxc-project/oxc#24044). + 'no-inner-declarations': ['warn', 'functions', { namespaces: 'allow' }], }, }, ], @@ -96,9 +95,31 @@ export default defineConfig({ 'unicorn/no-thenable': 'off', // The `children` prop is an accepted pattern in this codebase. 'react/no-children-prop': 'off', + // The ESLint rules flag callbacks only in the `disallow-in-func` mode, but oxlint always + // flags them, hitting the common fetch-on-mount pattern (`fetch().then(() => setState())`). + 'react/no-did-mount-set-state': 'off', + 'react/no-did-update-set-state': 'off', // Enums occasionally alias a value. 'typescript/no-duplicate-enum-values': 'off', + // ------------------------------------- + // --- React Compiler-derived rules --- + // ------------------------------------- + + // On by default since oxlint 1.79. The repo has a few hundred pre-existing violations + // (ref reads during render, setState calls in effects, and similar), so they are kept off + // to make the oxlint update behavior-neutral. Re-enable rule by rule as the findings + // get triaged. + 'react/globals': 'off', + 'react/immutability': 'off', + 'react/preserve-manual-memoization': 'off', + 'react/purity': 'off', + 'react/refs': 'off', + 'react/set-state-in-effect': 'off', + 'react/static-components': 'off', + 'react/use-memo': 'off', + 'react/void-use-memo': 'off', + // ----------------- // --- Stylistic --- // ----------------- diff --git a/packages/expo-module-scripts/package.json b/packages/expo-module-scripts/package.json index 5ce3c92b261f13..366dacd1b03a91 100644 --- a/packages/expo-module-scripts/package.json +++ b/packages/expo-module-scripts/package.json @@ -103,7 +103,7 @@ "jest-expo": "workspace:~56.0.4", "jest-snapshot-prettier": "npm:prettier@^2", "jest-watch-typeahead": "2.2.1", - "oxlint-config-universe": "^0.0.3", + "oxlint-config-universe": "^0.2.0", "resolve-workspace-root": "^2.0.0", "typescript": "^6.0.2" }, diff --git a/packages/expo-modules-core/CHANGELOG.md b/packages/expo-modules-core/CHANGELOG.md index 262a420cbcdc8a..08d2c2b61ee2c2 100644 --- a/packages/expo-modules-core/CHANGELOG.md +++ b/packages/expo-modules-core/CHANGELOG.md @@ -22,6 +22,7 @@ ### 🐛 Bug fixes +- [iOS][Android] Fixed a `matchContents` `RNHostView` and the `matchContents` host around it feeding each other's size back and forth, which grew the layout on every pass. ([#49483](https://github.com/expo/expo/pull/49483) by [@intergalacticspacehighway](https://github.com/intergalacticspacehighway)) - [iOS] Fixed `matchContents` hosts sometimes being laid out at a stale size. Regression from [#48059](https://github.com/expo/expo/pull/48059). ([#49211](https://github.com/expo/expo/pull/49211) by [@intergalacticspacehighway](https://github.com/intergalacticspacehighway)) - [iOS] Fixed an infinite main-thread layout loop (frozen UI, watchdog kill on backgrounding) when a SwiftUI host with `matchContents` and Yoga persistently disagree on the content size, e.g. with the Button Shapes accessibility setting enabled. Synchronous size commits are now budgeted per run-loop turn; over-budget updates are coalesced on the view and committed asynchronously on the next turn, and no-op size updates no longer dirty the layout. ([#48058](https://github.com/expo/expo/issues/48058), [#48059](https://github.com/expo/expo/pull/48059) by [@focux](https://github.com/focux)) - [iOS] Fixed the `ExpoModulesProvider` lookup missing the generated class when the app `name` starts with a digit, which registered no native modules and left release builds on a blank screen. ([#48793](https://github.com/expo/expo/pull/48793) by [@expo-bot](https://github.com/expo-bot)) diff --git a/packages/expo-modules-core/common/cpp/fabric/ExpoViewComponentDescriptor.h b/packages/expo-modules-core/common/cpp/fabric/ExpoViewComponentDescriptor.h index 9a1fc6bf61addd..2790724066f857 100644 --- a/packages/expo-modules-core/common/cpp/fabric/ExpoViewComponentDescriptor.h +++ b/packages/expo-modules-core/common/cpp/fabric/ExpoViewComponentDescriptor.h @@ -30,6 +30,32 @@ class ExpoViewComponentDescriptor return std::static_pointer_cast(this->flavor_)->c_str(); } + static bool isRNHostView(const react::Props::Shared &props) { + // Currently, only RNHostView can have `sizesToContent` set to true. + return ShadowNodeType::sizesToContent(props); + } + + std::shared_ptr createShadowNode( + const facebook::react::ShadowNodeFragment &fragment, + const facebook::react::ShadowNodeFamily::Shared &family + ) const override { + if (!isRNHostView(fragment.props)) { + return facebook::react::ConcreteComponentDescriptor::createShadowNode( + fragment, family); + } + + // Treat RNHostView as a leaf node and measurable node in Yoga, so that it can be measured by its children. + auto traits = this->getTraits(); + traits.set(facebook::react::ShadowNodeTraits::Trait::LeafYogaNode); + traits.set(facebook::react::ShadowNodeTraits::Trait::MeasurableYogaNode); + + auto shadowNode = std::make_shared(fragment, family, traits); + + this->adopt(*shadowNode); + + return shadowNode; + } + void adopt(facebook::react::ShadowNode &shadowNode) const override { react_native_assert(dynamic_cast(&shadowNode)); @@ -81,6 +107,21 @@ class ExpoViewComponentDescriptor // Updates yoga style from props and sets the node dirty snode->updateYogaProps(); } + + if (isRNHostView(snode->getProps())) { + auto const &props = *std::static_pointer_cast( + snode->getProps()); + auto &style = const_cast(props.yogaStyle); + + // If RNHostView has align self set to auto or stretch, we should override it to flex-start so that the node can size itself to its content + auto const alignSelf = style.alignSelf(); + + if (alignSelf == facebook::yoga::Align::Auto || alignSelf == facebook::yoga::Align::Stretch) { + style.setAlignSelf(facebook::yoga::Align::FlexStart); + snode->updateYogaProps(); + } + } + facebook::react::ConcreteComponentDescriptor::adopt(shadowNode); } }; diff --git a/packages/expo-modules-core/common/cpp/fabric/ExpoViewShadowNode.h b/packages/expo-modules-core/common/cpp/fabric/ExpoViewShadowNode.h index 9354e34f64abc6..ffd0f34830831e 100644 --- a/packages/expo-modules-core/common/cpp/fabric/ExpoViewShadowNode.h +++ b/packages/expo-modules-core/common/cpp/fabric/ExpoViewShadowNode.h @@ -5,6 +5,11 @@ #ifdef __cplusplus #include +#include +#include +#include + +#include #include "ContentOriginRegistry.h" #include "ExpoViewEventEmitter.h" @@ -77,7 +82,112 @@ class ExpoViewShadowNode : public facebook::react::ConcreteViewShadowNode< return {.x = contentOrigin->x - ownOrigin.x, .y = contentOrigin->y - ownOrigin.y}; } + // Currently, only `RNHostView` declares `expoInternalSizeFromChildren` + static bool sizesToContent(const react::Props::Shared &props) { + auto const *viewProps = dynamic_cast(props.get()); + + if (viewProps == nullptr) { + return false; + } + + auto const it = viewProps->propsMap.find("expoInternalSizeFromChildren"); + return it != viewProps->propsMap.end() && it->second.isBool() && it->second.getBool(); + } + + // Yoga calls this method for RNHostView when it has matchContents set + react::Size measureContent( + const react::LayoutContext &layoutContext, + const react::LayoutConstraints &layoutConstraints + ) const override { + // Return default behavior when RNHostView does not have `sizesToContent` set to true + if (!sizesToContent(this->getProps())) { + return ConcreteViewShadowNode::measureContent(layoutContext, layoutConstraints); + } + + auto const *content = hostedContent(); + + if (content == nullptr) { + return {}; + } + + return content->measure(layoutContext, hostedContentConstraints(*content)); + } + + // We override this so RNHostView can lay out it's children + // We marked it as a Leaf node so we need to manually lay out the hosted content + void layout(react::LayoutContext layoutContext) override { + ConcreteViewShadowNode::layout(layoutContext); + + if (!sizesToContent(this->getProps())) { + return; + } + + auto const *content = hostedContent(); + + if (content == nullptr) { + return; + } + + // Use the same constraint that was used to measure the content, so that the layout is consistent with the measurement + auto const clonedContent = content->clone({}); + static_cast(*clonedContent).layoutTree( + layoutContext, + hostedContentConstraints(*content) + ); + + this->replaceChild(*content, clonedContent, 0); + + if (layoutContext.affectedNodes != nullptr) { + layoutContext.affectedNodes->push_back( + static_cast(clonedContent.get())); + } + } + private: + const react::LayoutableShadowNode *hostedContent() const { + auto const &children = this->getChildren(); + + return children.empty() + ? nullptr + : dynamic_cast(children.front().get()); + } + + react::LayoutDirection resolvedLayoutDirection() const { + return YGNodeLayoutGetDirection(&this->yogaNode_) == YGDirectionRTL + ? react::LayoutDirection::RightToLeft + : react::LayoutDirection::LeftToRight; + } + + react::LayoutConstraints hostedContentConstraints(const react::ShadowNode &content) const { + react::LayoutConstraints constraints{}; + constraints.layoutDirection = resolvedLayoutDirection(); + + auto const *contentProps = dynamic_cast(content.getProps().get()); + + if (contentProps == nullptr) { + return constraints; + } + + auto const &style = contentProps->yogaStyle; + + constrainToPoints(style.minDimension(facebook::yoga::Dimension::Width), + constraints.minimumSize.width); + constrainToPoints(style.minDimension(facebook::yoga::Dimension::Height), + constraints.minimumSize.height); + constrainToPoints(style.maxDimension(facebook::yoga::Dimension::Width), + constraints.maximumSize.width); + constrainToPoints(style.maxDimension(facebook::yoga::Dimension::Height), + constraints.maximumSize.height); + + return constraints; + } + + static void constrainToPoints(facebook::yoga::StyleSizeLength length, react::Float &constraint) { + if (length.isPoints() && length.value().isDefined()) { + constraint = std::max(0, length.value().unwrap()); + } + } + void initialize() noexcept { auto &viewProps = static_cast(*this->props_); diff --git a/packages/expo-observe/src/__tests__/registerIntegration.test.native.ts b/packages/expo-observe/src/__tests__/registerIntegration.test.native.ts index 4c96bfb3b97c1f..aa794243287df3 100644 --- a/packages/expo-observe/src/__tests__/registerIntegration.test.native.ts +++ b/packages/expo-observe/src/__tests__/registerIntegration.test.native.ts @@ -1,8 +1,6 @@ /* eslint-disable @typescript-eslint/no-require-imports */ import type { ObserveModuleEvents } from '../types'; -export {}; - const CONFIGURE = 'configure' satisfies keyof ObserveModuleEvents; const configureListeners = new Set< (payload: Parameters[0]) => void diff --git a/packages/expo-router/src/global-state/useNavigationTreeReducer.ts b/packages/expo-router/src/global-state/useNavigationTreeReducer.ts index 97ab8c479646d6..ba7374d82d8e66 100644 --- a/packages/expo-router/src/global-state/useNavigationTreeReducer.ts +++ b/packages/expo-router/src/global-state/useNavigationTreeReducer.ts @@ -377,6 +377,9 @@ export function useNavigationTreeReducer({ previousRegistryRef.current = registry; for (const [stateKey, entry] of previousRegistry) { if (!registry.has(stateKey) && entry.routeNode) { + // This runs inside an effect; the rule doesn't recognize the `useClientLayoutEffect` + // wrapper as one. + // oxlint-disable-next-line react-hooks/rules-of-hooks process({ type: 'NAVIGATOR_UNMOUNTED', stateKey, diff --git a/packages/expo-ui/CHANGELOG.md b/packages/expo-ui/CHANGELOG.md index 4862074f544b90..bd275ffb154674 100644 --- a/packages/expo-ui/CHANGELOG.md +++ b/packages/expo-ui/CHANGELOG.md @@ -28,6 +28,7 @@ ### 🐛 Bug fixes +- [iOS][Android] Fixed a `matchContents` `RNHostView` inside a `matchContents` `Host` growing the layout on every pass. ([#49483](https://github.com/expo/expo/pull/49483) by [@intergalacticspacehighway](https://github.com/intergalacticspacehighway)) - [Android] Fix a drag that starts on a hosted `TextInput` not scrolling the `ScrollView` around it. React Native's text input asks its ancestors not to intercept the gesture, then releases them one move later, and Jetpack Compose read that release as "Compose claimed the gesture" and cancelled the hosted subtree. `RNHostView` no longer passes such a release on to Compose. - [Android] Fix the system status bar and navigation bar turning light while a `BottomSheet` or `ModalBottomSheet` with a custom dark background is open. A custom container color matches no color-scheme role, so the default content color fell back to black and Material3 themed the sheet window's system bars from it. The default content color is now derived from the container color's luminance, so it also contrasts with a custom background. ([#49394](https://github.com/expo/expo/pull/49394) by [@expo-bot](https://github.com/expo-bot)) - [iOS] Fixed a crash when a focused `TextField` or `SecureField` is unmounted inside a list row that is being removed, for example closing a modal with the keyboard up on a field nested in `SwipeActions`. Hosted text inputs now blur before React removes their native views. ([#49348](https://github.com/expo/expo/issues/49348) by [@nishan](https://github.com/intergalacticspacehighway)) ([#49357](https://github.com/expo/expo/pull/49357) by [@expo-tuft[bot]](https://github.com/apps/expo-tuft)) diff --git a/packages/expo-ui/android/src/main/java/expo/modules/ui/RNHostView.kt b/packages/expo-ui/android/src/main/java/expo/modules/ui/RNHostView.kt index 299a25c6578e1e..bbbcf8b0cbb755 100644 --- a/packages/expo-ui/android/src/main/java/expo/modules/ui/RNHostView.kt +++ b/packages/expo-ui/android/src/main/java/expo/modules/ui/RNHostView.kt @@ -41,6 +41,8 @@ import expo.modules.kotlin.views.OptimizedComposeProps @OptimizedComposeProps internal data class RNHostViewProps( val matchContents: MutableState = mutableStateOf(null), + // Adds LeafNode and MeasurableYogaNode trait in Shadow node + val expoInternalSizeFromChildren: MutableState = mutableStateOf(null), val modifiers: ModifierList = emptyList() ) : ComposeProps diff --git a/packages/expo-ui/ios/RNHostView.swift b/packages/expo-ui/ios/RNHostView.swift index dbb1882b1d5004..0b1a70981bd39e 100644 --- a/packages/expo-ui/ios/RNHostView.swift +++ b/packages/expo-ui/ios/RNHostView.swift @@ -5,6 +5,10 @@ import ExpoModulesCore internal final class RNHostViewProps: ExpoSwiftUI.ViewProps { @Field var matchContents: Bool = false + /** + Adds LeafNode and MeasurableYogaNode trait in Shadow node + */ + @Field var expoInternalSizeFromChildren: Bool = false /** Whether this view owns its subtree's touches and is the origin its content is measured from. Set by the JavaScript side for content presented in its own view controller — see `RNHostView.tsx`, diff --git a/packages/expo-ui/src/jetpack-compose/RNHostView/index.tsx b/packages/expo-ui/src/jetpack-compose/RNHostView/index.tsx index 298f88f9cc7d70..86de11d2b6d982 100644 --- a/packages/expo-ui/src/jetpack-compose/RNHostView/index.tsx +++ b/packages/expo-ui/src/jetpack-compose/RNHostView/index.tsx @@ -1,5 +1,6 @@ import { requireNativeView } from 'expo'; import type { ReactElement, ComponentType } from 'react'; +import type { LayoutChangeEvent } from 'react-native'; import { PresentedContentContext, useIsPresentedInOwnWindow } from '../../PresentedContentContext'; import type { ModifierConfig } from '../../types'; @@ -14,6 +15,11 @@ export interface RNHostProps extends PrimitiveBaseProps { * @default false */ matchContents?: boolean; + /** + * Called on mount and whenever this view's layout in the React Native view tree changes. + * With `matchContents`, the reported size is the one measured from the hosted view. + */ + onLayout?: (event: LayoutChangeEvent) => void; /** * The RN View to be hosted. */ @@ -26,6 +32,11 @@ export interface RNHostProps extends PrimitiveBaseProps { type NativeRNHostProps = RNHostProps & { layoutRoot: boolean; + /** + * Internal. Drives the shadow node's content measurement, see + * `ExpoViewShadowNode::sizesToContent`. + */ + expoInternalSizeFromChildren?: boolean; }; const NativeRNHostView: ComponentType = requireNativeView( 'ExpoUI', @@ -39,6 +50,7 @@ function transformProps(props: RNHostProps, layoutRoot: boolean): NativeRNHostPr ...(modifiers ? createViewModifierEventListener(modifiers) : undefined), ...restProps, layoutRoot, + expoInternalSizeFromChildren: props.matchContents, }; } diff --git a/packages/expo-ui/src/swift-ui/RNHostView.tsx b/packages/expo-ui/src/swift-ui/RNHostView.tsx index 2748ecd6bd0936..ac281cc21c6e8a 100644 --- a/packages/expo-ui/src/swift-ui/RNHostView.tsx +++ b/packages/expo-ui/src/swift-ui/RNHostView.tsx @@ -1,4 +1,5 @@ import { requireNativeView } from 'expo'; +import type { LayoutChangeEvent } from 'react-native'; import { PresentedContentContext, useIsPresentedInOwnWindow } from '../PresentedContentContext'; @@ -13,6 +14,7 @@ export interface RNHostViewProps { * @default false */ matchContents?: boolean; + onLayout?: (event: LayoutChangeEvent) => void; /** * The RN View to be hosted. */ @@ -30,6 +32,7 @@ export function RNHostView(props: RNHostViewProps) { diff --git a/packages/expo-ui/src/universal/RNHostView/index.android.tsx b/packages/expo-ui/src/universal/RNHostView/index.android.tsx index d33177007f76d8..9c9199d800991b 100644 --- a/packages/expo-ui/src/universal/RNHostView/index.android.tsx +++ b/packages/expo-ui/src/universal/RNHostView/index.android.tsx @@ -15,6 +15,7 @@ export function RNHostView({ disabled, hidden, testID, + onLayout, modifiers: extraModifiers, }: RNHostViewProps) { const modifiers = transformToModifiers( @@ -24,7 +25,7 @@ export function RNHostView({ ); return ( - + {children} ); diff --git a/packages/expo-ui/src/universal/RNHostView/index.ios.tsx b/packages/expo-ui/src/universal/RNHostView/index.ios.tsx index 4e6f8807368702..bcf6687138679d 100644 --- a/packages/expo-ui/src/universal/RNHostView/index.ios.tsx +++ b/packages/expo-ui/src/universal/RNHostView/index.ios.tsx @@ -5,8 +5,12 @@ import type { RNHostViewProps } from './types'; /** * Hosts React Native views inside SwiftUI views. */ -export function RNHostView({ children, matchContents }: RNHostViewProps) { - return {children}; +export function RNHostView({ children, matchContents, onLayout }: RNHostViewProps) { + return ( + + {children} + + ); } export * from './types'; diff --git a/packages/expo-ui/src/universal/RNHostView/index.tsx b/packages/expo-ui/src/universal/RNHostView/index.tsx index 9dd7f56dd5d17e..14614c0fd2de06 100644 --- a/packages/expo-ui/src/universal/RNHostView/index.tsx +++ b/packages/expo-ui/src/universal/RNHostView/index.tsx @@ -20,12 +20,14 @@ export function RNHostView({ hidden = false, testID, matchContents = false, + onLayout, }: RNHostViewProps) { useUniversalLifecycle(onAppear, onDisappear); return ( void; + /** * The React Native view to host. */ diff --git a/packages/expo-updates/CHANGELOG.md b/packages/expo-updates/CHANGELOG.md index 098359201dee71..658c2ab6846cbc 100644 --- a/packages/expo-updates/CHANGELOG.md +++ b/packages/expo-updates/CHANGELOG.md @@ -44,6 +44,9 @@ - [iOS] Resolved the reload screen's window through the shared scene geometry helper. ([#48172](https://github.com/expo/expo/pull/48172) by [@alanjhughes](https://github.com/alanjhughes)) - Removed Quick and Nimble in favor of Swift Testing. ([#48530](https://github.com/expo/expo/pull/48530) by [@tsapeta](https://github.com/tsapeta)) - [iOS] Link `libc++` in the test spec so the unit test bundle resolves the C++ symbols it pulls from `ExpoModulesCore`. ([#48762](https://github.com/expo/expo/pull/48762) by [@alanjhughes](https://github.com/alanjhughes)) +- [Android] Run the build data consistency check inside the startup procedure so it no longer queries the database on the main thread. ([#49374](https://github.com/expo/expo/pull/49374) by [@alanjhughes](https://github.com/alanjhughes)) +- [Android] Disallow main thread queries on the updates database. ([#49375](https://github.com/expo/expo/pull/49375) by [@alanjhughes](https://github.com/alanjhughes)) +- [Android] Remove the `DatabaseHolder` wrapper and pass `UpdatesDatabase` directly, since its lock released before any query ran and provided no real serialization. ([#49376](https://github.com/expo/expo/pull/49376) by [@alanjhughes](https://github.com/alanjhughes)) ## 57.0.11 - 2026-07-29 diff --git a/packages/expo-updates/android/src/main/java/expo/modules/updates/EnabledUpdatesController.kt b/packages/expo-updates/android/src/main/java/expo/modules/updates/EnabledUpdatesController.kt index 95495df3bcd039..3b3e503c976c3d 100644 --- a/packages/expo-updates/android/src/main/java/expo/modules/updates/EnabledUpdatesController.kt +++ b/packages/expo-updates/android/src/main/java/expo/modules/updates/EnabledUpdatesController.kt @@ -11,8 +11,6 @@ import com.facebook.react.devsupport.interfaces.DevSupportManager import expo.modules.easclient.EASClientID import expo.modules.kotlin.exception.CodedException import expo.modules.kotlin.exception.toCodedException -import expo.modules.updates.db.BuildData -import expo.modules.updates.db.DatabaseHolder import expo.modules.updates.db.UpdatesDatabase import expo.modules.updates.db.entity.UpdateEntity import expo.modules.updates.events.IUpdatesEventManager @@ -77,9 +75,9 @@ class EnabledUpdatesController( EASClientID(context).uuid.toString(), updatesConfiguration, logger, - databaseHolder.database + database ) - private val databaseHolder = DatabaseHolder(UpdatesDatabase.getInstance(context, Dispatchers.IO)) + private val database = UpdatesDatabase.getInstance(context, Dispatchers.IO) private val startupFinishedDeferred = CompletableDeferred() private val startupFinishedMutex = Mutex() override val reloadScreenManager = ReloadScreenManager() @@ -121,7 +119,7 @@ class EnabledUpdatesController( private val startupProcedure = StartupProcedure( context, updatesConfiguration, - databaseHolder, + database, updatesDirectory, fileDownloader, selectionPolicy, @@ -186,10 +184,6 @@ class EnabledUpdatesController( purgeUpdatesLogsOlderThanOneDay() - if (!updatesConfiguration.hasUpdatesOverride) { - BuildData.ensureBuildDataIsConsistent(updatesConfiguration, databaseHolder.database) - } - stateMachine.queueExecution(startupProcedure) } @@ -199,7 +193,7 @@ class EnabledUpdatesController( weakActivity, updatesConfiguration, logger, - databaseHolder, + database, updatesDirectory, fileDownloader, selectionPolicy, @@ -255,14 +249,14 @@ class EnabledUpdatesController( } override suspend fun checkForUpdate() = suspendCancellableCoroutine { continuation -> - val procedure = CheckForUpdateProcedure(context, updatesConfiguration, databaseHolder, logger, fileDownloader, selectionPolicy, launchedUpdate) { + val procedure = CheckForUpdateProcedure(context, updatesConfiguration, database, logger, fileDownloader, selectionPolicy, launchedUpdate) { continuation.resume(it) } stateMachine.queueExecution(procedure) } override suspend fun fetchUpdate() = suspendCancellableCoroutine { continuation -> - val procedure = FetchUpdateProcedure(context, updatesConfiguration, logger, databaseHolder, updatesDirectory, fileDownloader, selectionPolicy, launchedUpdate, controllerScope) { + val procedure = FetchUpdateProcedure(context, updatesConfiguration, logger, database, updatesDirectory, fileDownloader, selectionPolicy, launchedUpdate, controllerScope) { continuation.resume(it) } stateMachine.queueExecution(procedure) @@ -272,7 +266,7 @@ class EnabledUpdatesController( controllerScope.launch { try { val result = ManifestMetadata.getExtraParams( - databaseHolder.database, + database, updatesConfiguration ) val resultMap = when (result) { @@ -298,7 +292,7 @@ class EnabledUpdatesController( controllerScope.launch { try { ManifestMetadata.setExtraParam( - databaseHolder.database, + database, updatesConfiguration, key, value diff --git a/packages/expo-updates/android/src/main/java/expo/modules/updates/UpdatesDevLauncherController.kt b/packages/expo-updates/android/src/main/java/expo/modules/updates/UpdatesDevLauncherController.kt index 6b891b209dec69..1bebd1f1c109ec 100644 --- a/packages/expo-updates/android/src/main/java/expo/modules/updates/UpdatesDevLauncherController.kt +++ b/packages/expo-updates/android/src/main/java/expo/modules/updates/UpdatesDevLauncherController.kt @@ -8,7 +8,6 @@ import com.facebook.react.bridge.ReactContext import com.facebook.react.devsupport.interfaces.DevSupportManager import expo.modules.easclient.EASClientID import expo.modules.kotlin.exception.CodedException -import expo.modules.updates.db.DatabaseHolder import expo.modules.updates.db.Reaper import expo.modules.updates.db.UpdatesDatabase import expo.modules.updates.db.entity.AssetEntity @@ -72,7 +71,7 @@ class UpdatesDevLauncherController( private var previousUpdatesConfiguration: UpdatesConfiguration? = null private var updatesConfiguration: UpdatesConfiguration? = initialUpdatesConfiguration - private val databaseHolder = DatabaseHolder(UpdatesDatabase.getInstance(context, Dispatchers.IO)) + private val database = UpdatesDatabase.getInstance(context, Dispatchers.IO) private val controllerScope = CoroutineScope(SupervisorJob() + Dispatchers.IO) private var mSelectionPolicy: SelectionPolicy? = null @@ -186,13 +185,13 @@ class UpdatesDevLauncherController( EASClientID(context).uuid.toString(), updatesConfiguration!!, logger, - databaseHolder.database + database ) val loader = RemoteLoader( context, updatesConfiguration!!, logger, - databaseHolder.database, + database, fileDownloader, updatesDirectory, null, @@ -315,7 +314,7 @@ class UpdatesDevLauncherController( controllerScope ) try { - launcher.launch(databaseHolder.database) + launcher.launch(database) this@UpdatesDevLauncherController.launcher = launcher callback.onSuccess(object : UpdatesDevLauncherInterface.Update { override val manifest: JSONObject @@ -333,15 +332,12 @@ class UpdatesDevLauncherController( } } - private fun getDatabase(): UpdatesDatabase = databaseHolder.database - private fun runReaper() { controllerScope.launch { updatesConfiguration?.let { - val databaseLocal = getDatabase() Reaper.reapUnusedUpdates( it, - databaseLocal, + database, updatesDirectory, launchedUpdate, selectionPolicy diff --git a/packages/expo-updates/android/src/main/java/expo/modules/updates/db/DatabaseHolder.kt b/packages/expo-updates/android/src/main/java/expo/modules/updates/db/DatabaseHolder.kt deleted file mode 100644 index ab044e266f9b53..00000000000000 --- a/packages/expo-updates/android/src/main/java/expo/modules/updates/db/DatabaseHolder.kt +++ /dev/null @@ -1,33 +0,0 @@ -package expo.modules.updates.db - -import kotlinx.coroutines.runBlocking -import kotlinx.coroutines.sync.Mutex -import kotlinx.coroutines.sync.withLock - -/** - * Wrapper class that provides a rudimentary locking mechanism for the database. This allows us to - * control what high-level operations involving the database can occur simultaneously. Most classes - * should access [UpdatesDatabase] through this class. - * - */ -class DatabaseHolder(private val mDatabase: UpdatesDatabase) { - private val mutex = Mutex() - - // Less ideal but preserves accessing the database through a property - val database = runBlocking { - mutex.withLock { - mDatabase - } - } - - // For non blocking access to the database inside suspend functions - suspend fun withDatabase(block: suspend (UpdatesDatabase) -> T): T { - return mutex.withLock { - block(mDatabase) - } - } - - companion object { - private val TAG = DatabaseHolder::class.java.simpleName - } -} diff --git a/packages/expo-updates/android/src/main/java/expo/modules/updates/db/UpdatesDatabase.kt b/packages/expo-updates/android/src/main/java/expo/modules/updates/db/UpdatesDatabase.kt index a644889c3400e3..990f8ddfd463b3 100644 --- a/packages/expo-updates/android/src/main/java/expo/modules/updates/db/UpdatesDatabase.kt +++ b/packages/expo-updates/android/src/main/java/expo/modules/updates/db/UpdatesDatabase.kt @@ -38,10 +38,9 @@ import java.util.* * https://github.com/expo/expo/blob/main/packages/expo-updates/guides/migrations.md for step by * step instructions. * - * [DatabaseHolder] provides a rudimentary locking mechanism, and most other classes access the - * database through this class. This allows control over what high-level operations involving the - * database can occur simultaneously - e.g. we don't want to be trying to download a new update at - * the same time the [Reaper] is running. + * High-level operations involving the database (e.g. loading an update, running the [Reaper]) are + * serialized by the state machine's procedure queue; individual queries rely on Room's own thread + * safety. Queries must not run on the main thread. */ @Database( entities = [UpdateEntity::class, UpdateAssetEntity::class, AssetEntity::class, JSONDataEntity::class], @@ -81,7 +80,6 @@ abstract class UpdatesDatabase : RoomDatabase() { MIGRATION_11_12, MIGRATION_12_13 ) - .allowMainThreadQueries() .fallbackToDestructiveMigration() .build() diff --git a/packages/expo-updates/android/src/main/java/expo/modules/updates/loader/LoaderTask.kt b/packages/expo-updates/android/src/main/java/expo/modules/updates/loader/LoaderTask.kt index 5f5733d9a9a50c..18d724b8840239 100644 --- a/packages/expo-updates/android/src/main/java/expo/modules/updates/loader/LoaderTask.kt +++ b/packages/expo-updates/android/src/main/java/expo/modules/updates/loader/LoaderTask.kt @@ -5,8 +5,8 @@ import android.os.Handler import android.os.HandlerThread import expo.modules.updates.UpdatesConfiguration import expo.modules.updates.UpdatesUtils -import expo.modules.updates.db.DatabaseHolder import expo.modules.updates.db.Reaper +import expo.modules.updates.db.UpdatesDatabase import expo.modules.updates.db.entity.AssetEntity import expo.modules.updates.db.entity.UpdateEntity import expo.modules.updates.launcher.DatabaseLauncher @@ -46,7 +46,7 @@ import java.util.Date class LoaderTask( private val context: Context, private val configuration: UpdatesConfiguration, - private val databaseHolder: DatabaseHolder, + private val database: UpdatesDatabase, private val directory: File, private val fileDownloader: FileDownloader, private val selectionPolicy: SelectionPolicy, @@ -281,7 +281,6 @@ class LoaderTask( } private suspend fun launchFallbackUpdateFromDisk() { - val database = databaseHolder.database val launcher = DatabaseLauncher(context, configuration, directory, fileDownloader, selectionPolicy, logger, scope) candidateLauncher = launcher @@ -320,7 +319,6 @@ class LoaderTask( } private suspend fun launchRemoteUpdateInBackground() { - val database = databaseHolder.database callback.onRemoteCheckForUpdateStarted() val remoteLoader = RemoteLoader(context, configuration, logger, database, fileDownloader, directory, candidateLauncher?.launchedUpdate, scope) @@ -442,7 +440,6 @@ class LoaderTask( synchronized(this@LoaderTask) { val finalizedLaunchedUpdate = finalizedLauncher?.launchedUpdate if (finalizedLaunchedUpdate != null) { - val database = databaseHolder.database Reaper.reapUnusedUpdates( configuration, database, diff --git a/packages/expo-updates/android/src/main/java/expo/modules/updates/procedures/CheckForUpdateProcedure.kt b/packages/expo-updates/android/src/main/java/expo/modules/updates/procedures/CheckForUpdateProcedure.kt index 38953f0d8674fe..6dd0f6f42c1f43 100644 --- a/packages/expo-updates/android/src/main/java/expo/modules/updates/procedures/CheckForUpdateProcedure.kt +++ b/packages/expo-updates/android/src/main/java/expo/modules/updates/procedures/CheckForUpdateProcedure.kt @@ -4,7 +4,7 @@ import android.content.Context import expo.modules.core.logging.localizedMessageWithCauseLocalizedMessage import expo.modules.updates.IUpdatesController import expo.modules.updates.UpdatesConfiguration -import expo.modules.updates.db.DatabaseHolder +import expo.modules.updates.db.UpdatesDatabase import expo.modules.updates.db.entity.UpdateEntity import expo.modules.updates.loader.FileDownloader import expo.modules.updates.loader.LoaderTask @@ -20,7 +20,7 @@ import org.json.JSONObject class CheckForUpdateProcedure( private val context: Context, private val updatesConfiguration: UpdatesConfiguration, - private val databaseHolder: DatabaseHolder, + private val database: UpdatesDatabase, private val updatesLogger: UpdatesLogger, private val fileDownloader: FileDownloader, private val selectionPolicy: SelectionPolicy, @@ -34,7 +34,7 @@ class CheckForUpdateProcedure( val embeddedUpdate = EmbeddedManifestUtils.getEmbeddedUpdate(context, updatesConfiguration)?.updateEntity val extraHeaders = FileDownloader.getExtraHeadersForRemoteUpdateRequest( - databaseHolder.database, + database, updatesConfiguration, launchedUpdate, embeddedUpdate @@ -161,7 +161,7 @@ class CheckForUpdateProcedure( // only allow the update if it has had no launch failures. shouldLaunch = true update.updateEntity?.let { updateEntity -> - val storedUpdateEntity = databaseHolder.database.updateDao().loadUpdateWithId( + val storedUpdateEntity = database.updateDao().loadUpdateWithId( updateEntity.id ) storedUpdateEntity?.let { diff --git a/packages/expo-updates/android/src/main/java/expo/modules/updates/procedures/FetchUpdateProcedure.kt b/packages/expo-updates/android/src/main/java/expo/modules/updates/procedures/FetchUpdateProcedure.kt index c9ed656845e4de..f969f5bf896e3a 100644 --- a/packages/expo-updates/android/src/main/java/expo/modules/updates/procedures/FetchUpdateProcedure.kt +++ b/packages/expo-updates/android/src/main/java/expo/modules/updates/procedures/FetchUpdateProcedure.kt @@ -3,7 +3,6 @@ package expo.modules.updates.procedures import android.content.Context import expo.modules.updates.IUpdatesController import expo.modules.updates.UpdatesConfiguration -import expo.modules.updates.db.DatabaseHolder import expo.modules.updates.db.UpdatesDatabase import expo.modules.updates.db.entity.UpdateEntity import expo.modules.updates.loader.FileDownloader @@ -21,7 +20,7 @@ class FetchUpdateProcedure( private val context: Context, private val updatesConfiguration: UpdatesConfiguration, private val logger: UpdatesLogger, - private val databaseHolder: DatabaseHolder, + private val database: UpdatesDatabase, private val updatesDirectory: File, private val fileDownloader: FileDownloader, private val selectionPolicy: SelectionPolicy, @@ -34,7 +33,6 @@ class FetchUpdateProcedure( override suspend fun run(procedureContext: ProcedureContext) { procedureContext.processStateEvent(UpdatesStateEvent.Download()) - val database = databaseHolder.database try { val loaderResult = startRemoteLoader(database, procedureContext) processSuccessLoaderResult(loaderResult, procedureContext) @@ -103,7 +101,7 @@ class FetchUpdateProcedure( context, updatesConfiguration, logger, - databaseHolder.database, + database, selectionPolicy, updatesDirectory, launchedUpdate, diff --git a/packages/expo-updates/android/src/main/java/expo/modules/updates/procedures/RelaunchProcedure.kt b/packages/expo-updates/android/src/main/java/expo/modules/updates/procedures/RelaunchProcedure.kt index 5011fc68822e48..724e5c72a34e44 100644 --- a/packages/expo-updates/android/src/main/java/expo/modules/updates/procedures/RelaunchProcedure.kt +++ b/packages/expo-updates/android/src/main/java/expo/modules/updates/procedures/RelaunchProcedure.kt @@ -3,8 +3,8 @@ package expo.modules.updates.procedures import android.app.Activity import android.content.Context import expo.modules.updates.UpdatesConfiguration -import expo.modules.updates.db.DatabaseHolder import expo.modules.updates.db.Reaper +import expo.modules.updates.db.UpdatesDatabase import expo.modules.updates.launcher.DatabaseLauncher import expo.modules.updates.launcher.Launcher import expo.modules.updates.loader.FileDownloader @@ -26,7 +26,7 @@ class RelaunchProcedure( private val weakActivity: WeakReference?, private val updatesConfiguration: UpdatesConfiguration, private val logger: UpdatesLogger, - private val databaseHolder: DatabaseHolder, + private val database: UpdatesDatabase, private val updatesDirectory: File, private val fileDownloader: FileDownloader, private val selectionPolicy: SelectionPolicy, @@ -89,7 +89,7 @@ class RelaunchProcedure( try { Reaper.reapUnusedUpdates( updatesConfiguration, - databaseHolder.database, + database, updatesDirectory, getCurrentLauncher().launchedUpdate, selectionPolicy @@ -103,6 +103,6 @@ class RelaunchProcedure( } private suspend fun launchWith(newLauncher: DatabaseLauncher) { - newLauncher.launch(databaseHolder.database) + newLauncher.launch(database) } } diff --git a/packages/expo-updates/android/src/main/java/expo/modules/updates/procedures/StartupProcedure.kt b/packages/expo-updates/android/src/main/java/expo/modules/updates/procedures/StartupProcedure.kt index 346884db370517..c0de98e8ecdf8e 100644 --- a/packages/expo-updates/android/src/main/java/expo/modules/updates/procedures/StartupProcedure.kt +++ b/packages/expo-updates/android/src/main/java/expo/modules/updates/procedures/StartupProcedure.kt @@ -3,7 +3,8 @@ package expo.modules.updates.procedures import android.content.Context import com.facebook.react.devsupport.interfaces.DevSupportManager import expo.modules.updates.UpdatesConfiguration -import expo.modules.updates.db.DatabaseHolder +import expo.modules.updates.db.BuildData +import expo.modules.updates.db.UpdatesDatabase import expo.modules.updates.db.entity.AssetEntity import expo.modules.updates.db.entity.UpdateEntity import expo.modules.updates.errorrecovery.ErrorRecovery @@ -30,7 +31,7 @@ import java.io.File class StartupProcedure( private val context: Context, private val updatesConfiguration: UpdatesConfiguration, - private val databaseHolder: DatabaseHolder, + private val database: UpdatesDatabase, private val updatesDirectory: File, private val fileDownloader: FileDownloader, private val selectionPolicy: SelectionPolicy, @@ -72,7 +73,7 @@ class StartupProcedure( private val loaderTask = LoaderTask( context, updatesConfiguration, - databaseHolder, + database, updatesDirectory, fileDownloader, selectionPolicy, @@ -207,6 +208,9 @@ class StartupProcedure( override suspend fun run(procedureContext: ProcedureContext) { this.procedureContext = procedureContext procedureContext.processStateEvent(UpdatesStateEvent.StartStartup()) + if (!updatesConfiguration.hasUpdatesOverride) { + BuildData.ensureBuildDataIsConsistent(updatesConfiguration, database) + } initializeErrorRecovery() loaderTask.start() } @@ -243,7 +247,7 @@ class StartupProcedure( return } remoteLoadStatus = ErrorRecoveryDelegate.RemoteLoadStatus.NEW_UPDATE_LOADING - val remoteLoader = RemoteLoader(context, updatesConfiguration, logger, databaseHolder.database, fileDownloader, updatesDirectory, launchedUpdate, procedureScope) + val remoteLoader = RemoteLoader(context, updatesConfiguration, logger, database, fileDownloader, updatesDirectory, launchedUpdate, procedureScope) procedureScope.launch { try { val loaderResult = remoteLoader.load { updateResponse -> @@ -290,7 +294,7 @@ class StartupProcedure( } procedureScope.launch { val launchedUpdate = launchedUpdate ?: return@launch - databaseHolder.withDatabase { it.updateDao().incrementFailedLaunchCount(launchedUpdate) } + database.updateDao().incrementFailedLaunchCount(launchedUpdate) } } @@ -300,7 +304,7 @@ class StartupProcedure( } procedureScope.launch { val launchedUpdate = launchedUpdate ?: return@launch - databaseHolder.withDatabase { it.updateDao().incrementSuccessfulLaunchCount(launchedUpdate) } + database.updateDao().incrementSuccessfulLaunchCount(launchedUpdate) } } diff --git a/packages/expo-updates/android/src/test/java/expo/modules/updates/procedures/StartupProcedureTest.kt b/packages/expo-updates/android/src/test/java/expo/modules/updates/procedures/StartupProcedureTest.kt new file mode 100644 index 00000000000000..edd9b0b68d9345 --- /dev/null +++ b/packages/expo-updates/android/src/test/java/expo/modules/updates/procedures/StartupProcedureTest.kt @@ -0,0 +1,156 @@ +package expo.modules.updates.procedures + +import android.content.Context +import android.net.Uri +import androidx.room.Room +import androidx.test.core.app.ApplicationProvider +import expo.modules.updates.UpdatesConfiguration +import expo.modules.updates.UpdatesConfiguration.CheckAutomaticallyConfiguration +import expo.modules.updates.db.BuildData +import expo.modules.updates.db.UpdatesDatabase +import expo.modules.updates.db.entity.UpdateEntity +import expo.modules.updates.launcher.Launcher +import expo.modules.updates.loader.LoaderTask +import expo.modules.updates.logging.UpdatesLogger +import expo.modules.updates.statemachine.UpdatesStateEvent +import expo.modules.updates.statemachine.UpdatesStateValue +import io.mockk.coJustRun +import io.mockk.mockk +import io.mockk.mockkConstructor +import io.mockk.unmockkAll +import kotlinx.coroutines.test.runTest +import org.json.JSONObject +import org.junit.After +import org.junit.Assert +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import org.junit.rules.TemporaryFolder +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import java.util.Date +import java.util.UUID + +@RunWith(RobolectricTestRunner::class) +class StartupProcedureTest { + @get:Rule + val temporaryFolder = TemporaryFolder() + + private val context: Context = ApplicationProvider.getApplicationContext() + private lateinit var database: UpdatesDatabase + + private val procedureContext = object : StateMachineProcedure.ProcedureContext { + override fun processStateEvent(event: UpdatesStateEvent) {} + + @Deprecated("Avoid needing to access current state to know how to transition to next state") + override fun getCurrentState() = UpdatesStateValue.Idle + override fun resetStateAfterRestart() {} + override fun onComplete() {} + } + + @Before + fun setup() { + database = Room.inMemoryDatabaseBuilder(context, UpdatesDatabase::class.java) + .allowMainThreadQueries() + .build() + // Stop the procedure after the startup bookkeeping, before any loading work. + mockkConstructor(LoaderTask::class) + coJustRun { anyConstructed().start() } + } + + @After + fun teardown() { + database.close() + unmockkAll() + } + + @Test + fun `run clears updates when stored build data is inconsistent`() = runTest { + val config = createUpdatesConfiguration(channel = "default") + database.updateDao().insertUpdate(testUpdate(config.scopeKey)) + BuildData.setBuildDataInDatabase(database, createUpdatesConfiguration(channel = "preview")) + + runStartupProcedure(config) + + Assert.assertTrue(database.updateDao().loadAllUpdates().isEmpty()) + val storedBuildData = BuildData.getBuildDataFromDatabase(database, config.scopeKey)!! + Assert.assertTrue(BuildData.isBuildDataConsistent(config, storedBuildData)) + } + + @Test + fun `run keeps updates when stored build data is consistent`() = runTest { + val config = createUpdatesConfiguration(channel = "default") + database.updateDao().insertUpdate(testUpdate(config.scopeKey)) + BuildData.setBuildDataInDatabase(database, config) + + runStartupProcedure(config) + + Assert.assertEquals(1, database.updateDao().loadAllUpdates().size) + } + + @Test + fun `run skips the build data check when an updates override is set`() = runTest { + val config = createUpdatesConfiguration(channel = "default", hasUpdatesOverride = true) + database.updateDao().insertUpdate(testUpdate(config.scopeKey)) + BuildData.setBuildDataInDatabase(database, createUpdatesConfiguration(channel = "preview")) + + runStartupProcedure(config) + + Assert.assertEquals(1, database.updateDao().loadAllUpdates().size) + } + + private suspend fun runStartupProcedure(config: UpdatesConfiguration) { + val procedure = StartupProcedure( + context, + config, + database, + temporaryFolder.newFolder(".expo-internal"), + mockk(relaxed = true), + mockk(relaxed = true), + UpdatesLogger(context.filesDir), + object : StartupProcedure.StartupProcedureCallback { + override fun onFinished() {} + override fun onRequestRelaunch(shouldRunReaper: Boolean, callback: Launcher.LauncherCallback) {} + } + ) + procedure.run(procedureContext) + } + + private fun testUpdate(scopeKey: String) = UpdateEntity( + UUID.randomUUID(), + Date(), + "1.0.0", + scopeKey, + JSONObject(), + null, + null + ) + + private fun createUpdatesConfiguration( + channel: String, + hasUpdatesOverride: Boolean = false + ): UpdatesConfiguration { + val requestHeaders = mapOf("expo-channel-name" to channel) + return UpdatesConfiguration( + scopeKey = "test-scope", + updateUrl = Uri.parse("https://example.com"), + originalEmbeddedUpdateUrl = Uri.parse("https://example.com"), + runtimeVersionRaw = "1.0.0", + launchWaitMs = 0, + checkOnLaunch = CheckAutomaticallyConfiguration.ALWAYS, + hasEmbeddedUpdate = true, + originalHasEmbeddedUpdate = true, + requestHeaders = requestHeaders, + originalEmbeddedRequestHeaders = requestHeaders, + codeSigningCertificate = null, + codeSigningMetadata = emptyMap(), + codeSigningIncludeManifestResponseCertificateChain = false, + codeSigningAllowUnsignedManifests = false, + enableExpoUpdatesProtocolV0CompatibilityMode = false, + disableAntiBrickingMeasures = false, + enableBsdiffPatchSupport = false, + hasUpdatesOverride = hasUpdatesOverride, + cachedOverrideMap = emptyMap() + ) + } +} diff --git a/packages/expo-widgets/CHANGELOG.md b/packages/expo-widgets/CHANGELOG.md index 30ef3890c9b948..34ceea5f878748 100644 --- a/packages/expo-widgets/CHANGELOG.md +++ b/packages/expo-widgets/CHANGELOG.md @@ -25,6 +25,7 @@ ### 🐛 Bug fixes +- [iOS] Fix widget and Live Activity `Text` modifiers being applied twice. ([#49535](https://github.com/expo/expo/pull/49535) by [@gee1k](https://github.com/gee1k)) - Resolve deep `react-native/*` imports as empty modules when bundling widget layouts, fixing every widget failing with "Could not create context for layout evaluation" after `@expo/ui` 57.0.14 introduced such an import. ([#49491](https://github.com/expo/expo/pull/49491) by [@usmsam](https://github.com/usmsam)) - [iOS] Fix XCFramework precompilation failing on the unguarded `ActivityViewContext` parameter of `getLiveActivityEnvironment`, which requires iOS 16.1. ([#49076](https://github.com/expo/expo/pull/49076) by [@brentvatne](https://github.com/brentvatne)) - [iOS] Quote script-phase paths so iOS builds work from a project path containing a space. ([#48747](https://github.com/expo/expo/pull/48747) by [@expo-bot](https://github.com/expo-bot)) diff --git a/packages/expo-widgets/ios/Widgets/DynamicView.swift b/packages/expo-widgets/ios/Widgets/DynamicView.swift index 17f66095b8bdd2..53cd18faafeb78 100644 --- a/packages/expo-widgets/ios/Widgets/DynamicView.swift +++ b/packages/expo-widgets/ios/Widgets/DynamicView.swift @@ -48,7 +48,9 @@ public struct WidgetsDynamicView: View, ExpoSwiftUI.AnyChild { public var body: some View { switch node["type"] as? String { case "TextView": - render(TextView.self, TextViewProps.self, updateProps: updateChildren) + // TextView applies common modifiers internally so concatenated text keeps + // its SwiftUI.Text representation. Avoid applying those modifiers again. + render(TextView.self, TextViewProps.self, updateProps: updateChildren, wrapInUIBaseView: false) case "HStackView": render(HStackView.self, HStackViewProps.self, updateProps: updateChildren) case "VStackView": @@ -127,7 +129,12 @@ public struct WidgetsDynamicView: View, ExpoSwiftUI.AnyChild { // MARK: - Render Method @ViewBuilder - private func render(_ viewType: V.Type, _ propsType: P.Type, updateProps: ((_ initialProps: P) throws -> Void)? = nil) -> some View + private func render( + _ viewType: V.Type, + _ propsType: P.Type, + updateProps: ((_ initialProps: P) throws -> Void)? = nil, + wrapInUIBaseView: Bool = true + ) -> some View where P: UIBaseViewProps, V: ExpoSwiftUI.View, V.Props == P { // immediately invoked closure {}() here because we can't use 'do-catch' inside @ViewBuilder { @@ -136,7 +143,10 @@ public struct WidgetsDynamicView: View, ExpoSwiftUI.AnyChild { let props = try propsType.init(rawProps: rawProps, context: WidgetsContext.shared.context) try updateProps?(props) // TODO(@jakex7): Prevent unwanted transition when view is updated with new props - we want to have the same view instance recreated with new props instead of creating a new view instance and transitioning to it - return AnyView(UIBaseView(props: props).transition(.identity)) + if wrapInUIBaseView { + return AnyView(UIBaseView(props: props).transition(.identity)) + } + return AnyView(V(props: props).transition(.identity)) } return AnyView(EmptyView()) } catch { diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 000fb018ae2989..0180a10a7f3756 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -82,8 +82,8 @@ importers: specifier: ^0.55.0 version: 0.55.0 oxlint: - specifier: ^1.70.0 - version: 1.70.0 + specifier: ^1.79.0 + version: 1.79.0 prettier: specifier: ~3.8.3 version: 3.8.3 @@ -5109,8 +5109,8 @@ importers: specifier: 2.2.1 version: 2.2.1(jest@29.7.0(@types/node@22.20.1)(ts-node@10.9.2(@swc/core@1.15.18)(@types/node@22.20.1)(typescript@6.0.3))) oxlint-config-universe: - specifier: ^0.0.3 - version: 0.0.3(oxlint@1.70.0) + specifier: ^0.2.0 + version: 0.2.0(oxlint@1.79.0) resolve-workspace-root: specifier: ^2.0.0 version: 2.0.1 @@ -8543,124 +8543,124 @@ packages: cpu: [x64] os: [win32] - '@oxlint/binding-android-arm-eabi@1.70.0': - resolution: {integrity: sha512-zFh0P4cswmRvw6nkyb89dr18rRanuaCPAsEXsFDoQY8WdaquI8Pt4NWFjaMJg6L23cy5NeN8J9cBnREbWzZhaw==} + '@oxlint/binding-android-arm-eabi@1.79.0': + resolution: {integrity: sha512-TebFaaMklO/RXzTv7PucaCq9l3X6D1gA+C8H6K4njtjFOV+zWE9MKLpulcJZN9bzytbUbQIY0mZuz12nQ5Kv4Q==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm] os: [android] - '@oxlint/binding-android-arm64@1.70.0': - resolution: {integrity: sha512-qI8o4HZjeGiBrWv+pJv4lH0Yi2Gl/JSp/EumBUApezJprIKa5PS4nU0lQsQngtky8k+SplQIOjv6hwu0SSxeyg==} + '@oxlint/binding-android-arm64@1.79.0': + resolution: {integrity: sha512-KqqnOtAVgNsPPF0YSodkFZA1O80jcKoCZCTu3bgsszxA+MrMP9TLzfXitKjEj1FmrPprKDMdRDMmY3weESO9sg==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [android] - '@oxlint/binding-darwin-arm64@1.70.0': - resolution: {integrity: sha512-8KjgVVHI5F9nVwHCRwwA78Ty7zNKP4Wd9OeN5PSv3iu/F/u1RVXoOCgLhWqust6HmwQG6xc8c+RCyaWENy24+w==} + '@oxlint/binding-darwin-arm64@1.79.0': + resolution: {integrity: sha512-BVC2nsMzqQzRDPc5RhixkZ+m1p7iH4bxRRvqkbwDXX0PlQKm1BPy8J8cRjnAFafOq2QzI+BfO3vE8w2GZ3CBag==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [darwin] - '@oxlint/binding-darwin-x64@1.70.0': - resolution: {integrity: sha512-WVydssv5PSUBXFJTdNBWlmGkbNmvPGaFt/2SUT/EZRB6bq6bEOHmMlbnupZD5jmlEvi9+mZJHi8TCw15lyfSfQ==} + '@oxlint/binding-darwin-x64@1.79.0': + resolution: {integrity: sha512-p6Lm+snmhGuLKL1+CpCV8L6ijkE/qJzK2H2jG9+eKJT0n31RbY4FLsdhexekgP3bLpw4Kgde+9DZuDZQ4yIInA==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [x64] os: [darwin] - '@oxlint/binding-freebsd-x64@1.70.0': - resolution: {integrity: sha512-hJucmUf8OlinHNb1R7fI4Fw6WsAstOz7i8nmkWQfiHoZXtbufNm+MxiDTIMk1ggh2Ro4vLzgQ+bKvRY54MZoRA==} + '@oxlint/binding-freebsd-x64@1.79.0': + resolution: {integrity: sha512-qDMm0dXZnoHyRqSL4N4xUq82T4sqK5cbKSjvd/dF/YbMUXc2R1wEPf+vmA5S0qUmi0nwXfNbjXBtZaIqzQLIMg==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [x64] os: [freebsd] - '@oxlint/binding-linux-arm-gnueabihf@1.70.0': - resolution: {integrity: sha512-1BnS7wbCYDSXwWzJJ+mc3NURoha6m6m6RT5c6vgAY3oz7C3OVXP+S0awo2mRq97arrJkVvO3qRQfyAHL+76xtQ==} + '@oxlint/binding-linux-arm-gnueabihf@1.79.0': + resolution: {integrity: sha512-2od7s0nuKPzqyUZAWk9KkCyGg7eI9dwFPZg+20lB15fKFkVZ0c9ZFxqPfiBAyDTlTkh9stPI0t+JlPCqMbItVA==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm] os: [linux] - '@oxlint/binding-linux-arm-musleabihf@1.70.0': - resolution: {integrity: sha512-yKy/UdbR55+M2yEcuiV5DCNC/gdQAjr/GioUy50QwBzSrKm8ueWADqyRLS9Xk+qjNeCYGg6A8FvUBds56ttfqg==} + '@oxlint/binding-linux-arm-musleabihf@1.79.0': + resolution: {integrity: sha512-ZOQUjkzDnvlhSE3+tWC3YXx94MMl+sYMlwH+u1+YGApGHOJP/YAc8ZBRFOXZ6eOBmxtXAWuS/fBcdZr8qqNO1A==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm] os: [linux] - '@oxlint/binding-linux-arm64-gnu@1.70.0': - resolution: {integrity: sha512-0A5XJ4alvmqFUFP/4oYSyaO+qLto/HrKEWTSaegiVl+HOufFngK2BjYw9x4RbwBt/du5QG6l5q1zeWiJYYG5yg==} + '@oxlint/binding-linux-arm64-gnu@1.79.0': + resolution: {integrity: sha512-lu158FR4nGqGeRS3BQvtG85wRgU/Fy4MD5Cxp1hzJXizGiLo6u2742wJSCDKh8cFcZntvX7fcxlq4mMmfryH1g==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [linux] libc: [glibc] - '@oxlint/binding-linux-arm64-musl@1.70.0': - resolution: {integrity: sha512-JiylyurlB0CLSedNtx1gzv3FvfWPF1h/2Y3BJszPLNt5XQFlBsH5ke0Jle3iJb3uqu5m2e7A/DwzpuCAHdiU+A==} + '@oxlint/binding-linux-arm64-musl@1.79.0': + resolution: {integrity: sha512-mbpKQeE2aflTjddaHK7MP8KP/OFbUM++lt5M635ENM8IyIdK0jm2t9pb+2v9mVVIvhF6TqA4l7F79Pll1mi+uw==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [linux] libc: [musl] - '@oxlint/binding-linux-ppc64-gnu@1.70.0': - resolution: {integrity: sha512-J8VPG7I3/HmgaU4u8pNU2kFx2+0U+vPLS1dXFxXOaR/2TQ0f8AC7DRz0SRGRI1bfphnX2hVYTTtLuhL4nYKL+Q==} + '@oxlint/binding-linux-ppc64-gnu@1.79.0': + resolution: {integrity: sha512-WpGNua7gaxaHnpSDeog2ji8IDHn/QLPl9LPzwkR/FvVv58vT5BcXjRXnU+wbu3N75cpeha8CdC7ho/U2OIsB4g==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [ppc64] os: [linux] libc: [glibc] - '@oxlint/binding-linux-riscv64-gnu@1.70.0': - resolution: {integrity: sha512-N2+4lV2KLN+oXTIIIwmWDhwkrnvqf5oX7Hw0zPjk+RuIVgiBQSOlJWF7uQoFx2siEYX0ZQ5cfSbEAHm+J3t7Wg==} + '@oxlint/binding-linux-riscv64-gnu@1.79.0': + resolution: {integrity: sha512-tK1E93A5LVzISg4ngpKJnfTs7EqtIUceGI7MQ4GyDjJiLi8wPCkEyKlj2xkyKWZ1yzkDJyLHTBJ5/iFWRdnJvg==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [riscv64] os: [linux] libc: [glibc] - '@oxlint/binding-linux-riscv64-musl@1.70.0': - resolution: {integrity: sha512-1e2L7cFCvx9QDzq6NPP+0tABKb5z6nWHyddWTNKprEsjO9xNrAtPowuCGpjNXxkTdsMiZ4jc8YQ5SstZd4XK6g==} + '@oxlint/binding-linux-riscv64-musl@1.79.0': + resolution: {integrity: sha512-qhQvUIrngXivA2A9pQ+xPCychztn/5qUv7yS3gDwXv3w7Rag+eTeeXWmRyx+t7XsW5x6LuY/8AsTq36UgFIblg==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [riscv64] os: [linux] libc: [musl] - '@oxlint/binding-linux-s390x-gnu@1.70.0': - resolution: {integrity: sha512-Kwu/l/8GcYibCWA9m9N5pRXMIKVSsL/YbgpLzYkqDhWTiqdRfnNJ/+nqIKRKQiFbHWsdlHEhzMwruJK+qcEruA==} + '@oxlint/binding-linux-s390x-gnu@1.79.0': + resolution: {integrity: sha512-sv6AaVgU/eE6u+6WFiQVDcPPwTxP6IJMSB9k701W2r/r6Tx465e8vPvVyRxquNH4Vy6KwRNu90mVbxXJN8+5gg==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [s390x] os: [linux] libc: [glibc] - '@oxlint/binding-linux-x64-gnu@1.70.0': - resolution: {integrity: sha512-tap04CsHYOl0nSAQJfPNIuBxqEPB2HnhQqwaOXLg1jnp2XfRo8Fa814dA4QC4zpvTWXCjAAaCY1W5LOORkEQuQ==} + '@oxlint/binding-linux-x64-gnu@1.79.0': + resolution: {integrity: sha512-iFZL02deziHslb3jEX9KdqlAkYoo4fGyotchKDzdfK1f5mxlIBeiQeHhvK3iFpuEJSB4ma/qeFn9oxPiwnhUPQ==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [x64] os: [linux] libc: [glibc] - '@oxlint/binding-linux-x64-musl@1.70.0': - resolution: {integrity: sha512-hzJa/WgvtJpbBD9rgfy0qe+MjbxOXNUT0bfR1S6EQQzfTtBFA9xg5q8KSwRrQ2QfSS+TaP4j+4mVPQrfNc6UNg==} + '@oxlint/binding-linux-x64-musl@1.79.0': + resolution: {integrity: sha512-3DtZR2raqObnh7wXZoFYFd0Fw7skBvcb3f7A+/lkEiDuh8hrE6vv9b/62Qxao1a9/OeHLw/FcXlXzgsW9wTRFg==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [x64] os: [linux] libc: [musl] - '@oxlint/binding-openharmony-arm64@1.70.0': - resolution: {integrity: sha512-xbsaNSNzVSnaJACCUYr1HQMyY/Q/Q1LkePmHG3UvZPvGCYGNxrsZp9OmtA6ick8xH47ltRRbRrPCM1YXYcyC+A==} + '@oxlint/binding-openharmony-arm64@1.79.0': + resolution: {integrity: sha512-Oatt4GuA1WJkqzk2ozx4HrWROOi7opV3AKDw/U8qDIqeTqzsjn5K2x3REJMNjU3/KU/Bkq96Zi3CknaiDTaC/Q==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [openharmony] - '@oxlint/binding-win32-arm64-msvc@1.70.0': - resolution: {integrity: sha512-icAEsUI7JbW1TMRdEXV83mVAInhRVQYuuAlPpxdGwJ95chNdnCzjloRW8GglT0WvzOEZSio6fnYSk2DJ2Hv7LQ==} + '@oxlint/binding-win32-arm64-msvc@1.79.0': + resolution: {integrity: sha512-NAgZr9Qp8nIA9rpo0JEvwiabTF/2UVqBNnupBG9X4kxXcQoScJUTi+qHhvabb9s/thgj5wQ4XcIaJvb+ZMgoKw==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [arm64] os: [win32] - '@oxlint/binding-win32-ia32-msvc@1.70.0': - resolution: {integrity: sha512-FHMSWbVsPVs/f+Jcl04ws4JJ2wUnauyTzlpxWRG/lSO/8GpX08Fo2gQZqdA6CrRFI+zvkxl+N/KwJGWfUwYVZA==} + '@oxlint/binding-win32-ia32-msvc@1.79.0': + resolution: {integrity: sha512-+KyXjIvcpaXmWW/j9NNY5yWjrIVxaX18VyIheQy3jwc2GSYgpCr7MGI/HxIGQ/shAL5IWEKbhsqoMpAO5Stiog==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [ia32] os: [win32] - '@oxlint/binding-win32-x64-msvc@1.70.0': - resolution: {integrity: sha512-ptOlKwCz7n4AKs5VweMqG6DAg677FmKOK+vBkkL9DMNgFATIQ+upqUYBTOEwRQyRAx1ncGlPlXleV2hIcm3z4g==} + '@oxlint/binding-win32-x64-msvc@1.79.0': + resolution: {integrity: sha512-mEelcCMMBS57sIXh2veGMNy+pQwuGtcMxHxGIZWQ5Ba9pJ5jCCUFOZB9E2JhBaxGsURe+WGe0zJp4RVre52gpQ==} engines: {node: ^20.19.0 || >=22.12.0} cpu: [x64] os: [win32] @@ -13558,17 +13558,21 @@ packages: vite-plus: optional: true - oxlint-config-universe@0.0.3: - resolution: {integrity: sha512-EpKPtjNpDu2EfkmIsjGMAuM3QQ37fVaLdjg6sEK+lGf5QZpK+pX9DjG6F9zz29iLfdgvojAQveQZW6ViFbhd+Q==} + oxlint-config-universe@0.2.0: + resolution: {integrity: sha512-uGpKrct7Ny+NsU1QVcHfY3ONFu9q1ip26ymxwzNFgBPAhOqIPn8EM68GNy+G9AXf9usU4jbt14Kk5PUVBKFPPw==} peerDependencies: - oxlint: '>=1.59.0' + oxlint: '>=1.79.0' + oxlint-tsgolint: '>=7.0.2001' + peerDependenciesMeta: + oxlint-tsgolint: + optional: true - oxlint@1.70.0: - resolution: {integrity: sha512-D6JgHtzkhRwvEC+A0Nw5AEc5bk8x5i1pHzvZIEf/a0C4hOzmAACNGtkDGPyFaxxX3ZVGxCPeig3P3rMM8XU3/g==} + oxlint@1.79.0: + resolution: {integrity: sha512-hVJ9hq9m2unPS+Of4eJJgCPdIeCC+3DHEUX3tkmrPJr3OK2hz7PhXwgC+ZP71ZcYu8cCDEtQrqLxWNvxBppBVg==} engines: {node: ^20.19.0 || >=22.12.0} hasBin: true peerDependencies: - oxlint-tsgolint: '>=0.22.1' + oxlint-tsgolint: '>=7.0.2001' vite-plus: '*' peerDependenciesMeta: oxlint-tsgolint: @@ -17601,61 +17605,61 @@ snapshots: '@oxfmt/binding-win32-x64-msvc@0.55.0': optional: true - '@oxlint/binding-android-arm-eabi@1.70.0': + '@oxlint/binding-android-arm-eabi@1.79.0': optional: true - '@oxlint/binding-android-arm64@1.70.0': + '@oxlint/binding-android-arm64@1.79.0': optional: true - '@oxlint/binding-darwin-arm64@1.70.0': + '@oxlint/binding-darwin-arm64@1.79.0': optional: true - '@oxlint/binding-darwin-x64@1.70.0': + '@oxlint/binding-darwin-x64@1.79.0': optional: true - '@oxlint/binding-freebsd-x64@1.70.0': + '@oxlint/binding-freebsd-x64@1.79.0': optional: true - '@oxlint/binding-linux-arm-gnueabihf@1.70.0': + '@oxlint/binding-linux-arm-gnueabihf@1.79.0': optional: true - '@oxlint/binding-linux-arm-musleabihf@1.70.0': + '@oxlint/binding-linux-arm-musleabihf@1.79.0': optional: true - '@oxlint/binding-linux-arm64-gnu@1.70.0': + '@oxlint/binding-linux-arm64-gnu@1.79.0': optional: true - '@oxlint/binding-linux-arm64-musl@1.70.0': + '@oxlint/binding-linux-arm64-musl@1.79.0': optional: true - '@oxlint/binding-linux-ppc64-gnu@1.70.0': + '@oxlint/binding-linux-ppc64-gnu@1.79.0': optional: true - '@oxlint/binding-linux-riscv64-gnu@1.70.0': + '@oxlint/binding-linux-riscv64-gnu@1.79.0': optional: true - '@oxlint/binding-linux-riscv64-musl@1.70.0': + '@oxlint/binding-linux-riscv64-musl@1.79.0': optional: true - '@oxlint/binding-linux-s390x-gnu@1.70.0': + '@oxlint/binding-linux-s390x-gnu@1.79.0': optional: true - '@oxlint/binding-linux-x64-gnu@1.70.0': + '@oxlint/binding-linux-x64-gnu@1.79.0': optional: true - '@oxlint/binding-linux-x64-musl@1.70.0': + '@oxlint/binding-linux-x64-musl@1.79.0': optional: true - '@oxlint/binding-openharmony-arm64@1.70.0': + '@oxlint/binding-openharmony-arm64@1.79.0': optional: true - '@oxlint/binding-win32-arm64-msvc@1.70.0': + '@oxlint/binding-win32-arm64-msvc@1.79.0': optional: true - '@oxlint/binding-win32-ia32-msvc@1.70.0': + '@oxlint/binding-win32-ia32-msvc@1.79.0': optional: true - '@oxlint/binding-win32-x64-msvc@1.70.0': + '@oxlint/binding-win32-x64-msvc@1.79.0': optional: true '@parcel/watcher-android-arm64@2.5.6': @@ -23315,31 +23319,31 @@ snapshots: '@oxfmt/binding-win32-ia32-msvc': 0.55.0 '@oxfmt/binding-win32-x64-msvc': 0.55.0 - oxlint-config-universe@0.0.3(oxlint@1.70.0): + oxlint-config-universe@0.2.0(oxlint@1.79.0): dependencies: - oxlint: 1.70.0 + oxlint: 1.79.0 - oxlint@1.70.0: + oxlint@1.79.0: optionalDependencies: - '@oxlint/binding-android-arm-eabi': 1.70.0 - '@oxlint/binding-android-arm64': 1.70.0 - '@oxlint/binding-darwin-arm64': 1.70.0 - '@oxlint/binding-darwin-x64': 1.70.0 - '@oxlint/binding-freebsd-x64': 1.70.0 - '@oxlint/binding-linux-arm-gnueabihf': 1.70.0 - '@oxlint/binding-linux-arm-musleabihf': 1.70.0 - '@oxlint/binding-linux-arm64-gnu': 1.70.0 - '@oxlint/binding-linux-arm64-musl': 1.70.0 - '@oxlint/binding-linux-ppc64-gnu': 1.70.0 - '@oxlint/binding-linux-riscv64-gnu': 1.70.0 - '@oxlint/binding-linux-riscv64-musl': 1.70.0 - '@oxlint/binding-linux-s390x-gnu': 1.70.0 - '@oxlint/binding-linux-x64-gnu': 1.70.0 - '@oxlint/binding-linux-x64-musl': 1.70.0 - '@oxlint/binding-openharmony-arm64': 1.70.0 - '@oxlint/binding-win32-arm64-msvc': 1.70.0 - '@oxlint/binding-win32-ia32-msvc': 1.70.0 - '@oxlint/binding-win32-x64-msvc': 1.70.0 + '@oxlint/binding-android-arm-eabi': 1.79.0 + '@oxlint/binding-android-arm64': 1.79.0 + '@oxlint/binding-darwin-arm64': 1.79.0 + '@oxlint/binding-darwin-x64': 1.79.0 + '@oxlint/binding-freebsd-x64': 1.79.0 + '@oxlint/binding-linux-arm-gnueabihf': 1.79.0 + '@oxlint/binding-linux-arm-musleabihf': 1.79.0 + '@oxlint/binding-linux-arm64-gnu': 1.79.0 + '@oxlint/binding-linux-arm64-musl': 1.79.0 + '@oxlint/binding-linux-ppc64-gnu': 1.79.0 + '@oxlint/binding-linux-riscv64-gnu': 1.79.0 + '@oxlint/binding-linux-riscv64-musl': 1.79.0 + '@oxlint/binding-linux-s390x-gnu': 1.79.0 + '@oxlint/binding-linux-x64-gnu': 1.79.0 + '@oxlint/binding-linux-x64-musl': 1.79.0 + '@oxlint/binding-openharmony-arm64': 1.79.0 + '@oxlint/binding-win32-arm64-msvc': 1.79.0 + '@oxlint/binding-win32-ia32-msvc': 1.79.0 + '@oxlint/binding-win32-x64-msvc': 1.79.0 p-cancelable@2.1.1: {} diff --git a/tools/src/commands/GitHubMetricsCommand.test.ts b/tools/src/commands/GitHubMetricsCommand.test.ts index 5f9a5dd49cffc2..cfc87e75f6a6d5 100644 --- a/tools/src/commands/GitHubMetricsCommand.test.ts +++ b/tools/src/commands/GitHubMetricsCommand.test.ts @@ -1,12 +1,24 @@ import assert from 'node:assert/strict'; import { describe, it } from 'node:test'; -import { computeRunConclusionCounts } from './GitHubMetricsCommand'; +import { + computeRunConclusionCounts, + computeWorkflowStats, + formatSuccessRate, +} from './GitHubMetricsCommand'; function runs(conclusions: (string | null)[]) { return conclusions.map((conclusion) => ({ conclusion })); } +function repeat(conclusion: string | null, count: number) { + return Array.from({ length: count }, () => conclusion); +} + +function workflowRun(workflowId: number, name: string | null, filePath: string) { + return { workflow_id: workflowId, name, path: filePath, conclusion: 'success' }; +} + describe('computeRunConclusionCounts', () => { it('rates all-successful runs at 100%', () => { const result = computeRunConclusionCounts(runs(['success', 'success'])); @@ -15,27 +27,126 @@ describe('computeRunConclusionCounts', () => { it('counts cancelled runs as successful', () => { const result = computeRunConclusionCounts(runs(['success', 'cancelled', 'failure'])); - assert.equal(result.totalRuns - result.skippedRuns, 3); + assert.equal(result.resolvedRuns, 3); assert.equal(result.successRate, (2 / 3) * 100); }); it('excludes skipped runs from the success-rate denominator', () => { const result = computeRunConclusionCounts(runs(['skipped', 'skipped', 'skipped', 'success'])); assert.equal(result.skippedRuns, 3); - assert.equal(result.totalRuns - result.skippedRuns, 1); + assert.equal(result.resolvedRuns, 1); assert.equal(result.successRate, 100); }); it('does not divide by zero when every run was skipped', () => { const result = computeRunConclusionCounts(runs(['skipped', 'skipped'])); - assert.equal(result.totalRuns - result.skippedRuns, 0); + assert.equal(result.resolvedRuns, 0); assert.equal(result.successRate, 0); }); - it('leaves ambiguous conclusions (in progress, neutral, etc.) in the denominator', () => { + it('excludes unresolved runs (in progress, queued, neutral) from the denominator', () => { const result = computeRunConclusionCounts(runs([null, 'neutral', 'success'])); assert.equal(result.otherRuns, 2); - assert.equal(result.totalRuns - result.skippedRuns, 3); - assert.equal(result.successRate, (1 / 3) * 100); + assert.equal(result.resolvedRuns, 1); + assert.equal(result.successRate, 100); + }); + + it('reports the resolved success rate for a real week of runs', () => { + const result = computeRunConclusionCounts( + runs([ + ...repeat('success', 335), + ...repeat('failure', 31), + ...repeat('cancelled', 96), + ...repeat('skipped', 468), + ...repeat(null, 70), + ]) + ); + + assert.equal(result.totalRuns, 1000); + assert.equal(result.otherRuns, 70); + assert.equal(result.resolvedRuns, 462); + assert.equal(result.successRate.toFixed(1), '93.3'); + assert.notEqual(result.successRate.toFixed(1), '81.0'); + }); + + it('rates a failure-free set at 100% however many runs are still unresolved', () => { + const result = computeRunConclusionCounts( + runs(['success', 'cancelled', null, 'action_required', 'queued']) + ); + assert.equal(result.failedRuns, 0); + assert.equal(result.successRate, 100); + }); +}); + +describe('formatSuccessRate', () => { + it('has no rate to report when nothing resolved', () => { + const result = computeRunConclusionCounts(runs([null, null, 'action_required'])); + assert.equal(formatSuccessRate(result), 'N/A (no executed runs)'); + }); + + it('rounds the resolved rate to one decimal', () => { + const result = computeRunConclusionCounts( + runs([...repeat('success', 431), ...repeat('failure', 31), ...repeat(null, 70)]) + ); + assert.equal(formatSuccessRate(result), '93.3%'); + }); +}); + +describe('computeWorkflowStats', () => { + const verifyPath = '.github/workflows/verify-comment.yml'; + + it('groups runs of one workflow together despite dynamic run names', () => { + const stats = computeWorkflowStats([ + workflowRun(342612912, 'verify', verifyPath), + workflowRun(342612912, 'verify #49383 — expo-bot', verifyPath), + workflowRun(342612912, 'verify #48626 — brentvatne', verifyPath), + ]); + + assert.equal(stats.length, 1); + assert.equal(stats[0].name, 'verify'); + assert.equal(stats[0].totalRuns, 3); + }); + + it('keeps distinct workflows apart even when they share a name', () => { + const stats = computeWorkflowStats([ + workflowRun(1, 'test', '.github/workflows/test-suite.yml'), + workflowRun(2, 'test', '.github/workflows/test-tools.yml'), + ]); + + assert.equal(stats.length, 2); + }); + + it('falls back to the workflow file name when no run is named', () => { + const stats = computeWorkflowStats([ + workflowRun(1, null, verifyPath), + workflowRun(1, null, verifyPath), + ]); + + assert.deepEqual( + stats.map((workflow) => workflow.name), + ['verify-comment.yml'] + ); + }); + + it('breaks shortest-name ties alphabetically', () => { + const stats = computeWorkflowStats([ + workflowRun(1, 'beta', verifyPath), + workflowRun(1, 'alfa', verifyPath), + ]); + + assert.equal(stats[0].name, 'alfa'); + }); + + it('orders workflows by total runs, descending', () => { + const stats = computeWorkflowStats([ + workflowRun(1, 'once', '.github/workflows/once.yml'), + workflowRun(2, 'twice', '.github/workflows/twice.yml'), + workflowRun(2, 'twice', '.github/workflows/twice.yml'), + ]); + + assert.deepEqual( + stats.map((workflow) => workflow.name), + ['twice', 'once'] + ); }); }); diff --git a/tools/src/commands/GitHubMetricsCommand.ts b/tools/src/commands/GitHubMetricsCommand.ts index 337431c8e42818..2fd478e54c997f 100644 --- a/tools/src/commands/GitHubMetricsCommand.ts +++ b/tools/src/commands/GitHubMetricsCommand.ts @@ -64,7 +64,8 @@ interface WorkflowStats { cancelledRuns: number; skippedRuns: number; otherRuns: number; // timed_out, action_required, neutral, stale, in_progress, etc. - successRate: number; // percentage of (totalRuns - skippedRuns) that succeeded or were cancelled + resolvedRuns: number; // runs that reached an outcome: successful + failed + cancelled + successRate: number; // percentage of resolvedRuns that succeeded or were cancelled } interface CIMetrics { @@ -74,7 +75,8 @@ interface CIMetrics { cancelledRuns: number; skippedRuns: number; otherRuns: number; // timed_out, action_required, neutral, stale, in_progress, etc. - successRate: number; // percentage of (totalRuns - skippedRuns) that succeeded or were cancelled + resolvedRuns: number; // runs that reached an outcome: successful + failed + cancelled + successRate: number; // percentage of resolvedRuns that succeeded or were cancelled workflows: WorkflowStats[]; } @@ -360,9 +362,9 @@ export function computeRunConclusionCounts(runs: { conclusion: string | null }[] const skippedRuns = runs.filter((run) => run.conclusion === 'skipped').length; const otherRuns = totalRuns - successfulRuns - failedRuns - cancelledRuns - skippedRuns; - const executedRuns = totalRuns - skippedRuns; + const resolvedRuns = successfulRuns + failedRuns + cancelledRuns; const effectiveSuccessfulRuns = successfulRuns + cancelledRuns; - const successRate = executedRuns > 0 ? (effectiveSuccessfulRuns / executedRuns) * 100 : 0; + const successRate = resolvedRuns > 0 ? (effectiveSuccessfulRuns / resolvedRuns) * 100 : 0; return { totalRuns, @@ -371,10 +373,59 @@ export function computeRunConclusionCounts(runs: { conclusion: string | null }[] cancelledRuns, skippedRuns, otherRuns, + resolvedRuns, successRate, }; } +export function formatSuccessRate(stats: { resolvedRuns: number; successRate: number }): string { + return stats.resolvedRuns > 0 ? `${stats.successRate.toFixed(1)}%` : 'N/A (no executed runs)'; +} + +type WorkflowRunSummary = { + workflow_id: number; + name?: string | null; + path?: string | null; + conclusion: string | null; +}; + +/** + * Group runs per workflow, ordered by run count descending. + * + * Runs are grouped by `workflow_id` rather than by `name`: a workflow that sets `run-name:` gets a + * fresh name on every run (`verify`, `verify #49383 — expo-bot`, …), which would otherwise scatter + * one workflow across dozens of single-run rows. + */ +export function computeWorkflowStats(workflowRuns: WorkflowRunSummary[]): WorkflowStats[] { + const runsByWorkflowId = new Map(); + for (const run of workflowRuns) { + const runs = runsByWorkflowId.get(run.workflow_id); + if (runs) { + runs.push(run); + } else { + runsByWorkflowId.set(run.workflow_id, [run]); + } + } + + return Array.from(runsByWorkflowId.values()) + .map((runs) => ({ name: workflowLabel(runs), ...computeRunConclusionCounts(runs) })) + .sort((a, b) => b.totalRuns - a.totalRuns); +} + +/** + * The shortest run name of a group, which is the workflow's own name without any run-specific + * suffix. Falls back to the workflow file name for workflows whose runs are all unnamed. + */ +function workflowLabel(runs: WorkflowRunSummary[]): string { + const names = runs.flatMap((run) => (run.name ? [run.name] : [])); + if (names.length > 0) { + return names.sort((a, b) => a.length - b.length || a.localeCompare(b))[0]; + } + + const workflowPath = runs.find((run) => run.path)?.path; + return workflowPath ? path.basename(workflowPath) : 'unknown'; +} + /** * Fetch CI/CD workflow metrics for the given time period */ @@ -397,22 +448,9 @@ async function fetchCIMetrics( return data.workflow_runs; }); - const workflowMap = new Map(); - for (const run of workflowRuns) { - const workflowName = run.name ?? 'unknown'; - if (!workflowMap.has(workflowName)) { - workflowMap.set(workflowName, []); - } - workflowMap.get(workflowName)!.push(run); - } - - const workflows: WorkflowStats[] = Array.from(workflowMap.entries()) - .map(([name, runs]) => ({ name, ...computeRunConclusionCounts(runs) })) - .sort((a, b) => b.totalRuns - a.totalRuns); - return { ...computeRunConclusionCounts(workflowRuns), - workflows, + workflows: computeWorkflowStats(workflowRuns), }; } @@ -490,15 +528,6 @@ function generateMarkdownReport(metrics: MetricsData, options: MetricsOptions): const startDateStr = startDate.toISOString().split('T')[0]; const endDateStr = endDate.toISOString().split('T')[0]; - const formatSuccessRate = (stats: { - totalRuns: number; - skippedRuns: number; - successRate: number; - }): string => - stats.totalRuns - stats.skippedRuns > 0 - ? `${stats.successRate.toFixed(1)}%` - : 'N/A (no executed runs)'; - const report = `# GitHub Metrics Report **Repository:** ${owner}/${repo} @@ -552,15 +581,15 @@ function generateMarkdownReport(metrics: MetricsData, options: MetricsOptions): | Failed runs | ${metrics.ci.failedRuns} | ${metrics.ci.totalRuns > 0 ? ((metrics.ci.failedRuns / metrics.ci.totalRuns) * 100).toFixed(1) : '0.0'}% | | Cancelled runs (concurrency) | ${metrics.ci.cancelledRuns} | ${metrics.ci.totalRuns > 0 ? ((metrics.ci.cancelledRuns / metrics.ci.totalRuns) * 100).toFixed(1) : '0.0'}% | | Skipped runs (never executed) | ${metrics.ci.skippedRuns} | ${metrics.ci.totalRuns > 0 ? ((metrics.ci.skippedRuns / metrics.ci.totalRuns) * 100).toFixed(1) : '0.0'}% | -| Other runs (in progress, neutral, etc.) | ${metrics.ci.otherRuns} | ${metrics.ci.totalRuns > 0 ? ((metrics.ci.otherRuns / metrics.ci.totalRuns) * 100).toFixed(1) : '0.0'}% | -| **Success rate** | **${metrics.ci.successfulRuns + metrics.ci.cancelledRuns}/${metrics.ci.totalRuns - metrics.ci.skippedRuns}** | **${formatSuccessRate(metrics.ci)}** | +| Unresolved runs (in progress, queued, awaiting approval) | ${metrics.ci.otherRuns} | ${metrics.ci.totalRuns > 0 ? ((metrics.ci.otherRuns / metrics.ci.totalRuns) * 100).toFixed(1) : '0.0'}% | +| **Success rate** | **${metrics.ci.successfulRuns + metrics.ci.cancelledRuns}/${metrics.ci.resolvedRuns}** | **${formatSuccessRate(metrics.ci)}** | -> **Note:** Cancelled runs are counted as successful since they're typically due to concurrency settings (newer runs superseding older ones). Skipped runs are excluded from the success rate entirely, since a \`skipped\` conclusion means the workflow never executed (usually because an \`if:\` condition gated it, like a fork-only check). +> **Note:** Cancelled runs are counted as successful since they're typically due to concurrency settings (newer runs superseding older ones). Skipped runs are excluded from the success rate entirely, since a \`skipped\` conclusion means the workflow never executed (usually because an \`if:\` condition gated it, like a fork-only check). Unresolved runs are excluded for the same reason — they had no outcome yet when the report was generated, so counting them as failures would make the rate rise and fall with how much CI happened to be in flight. ### Workflow Breakdown -| Workflow | Total | Success | Failed | Cancelled | Skipped | Other | Success Rate | -|----------|-------|---------|--------|-----------|---------|-------|--------------| +| Workflow | Total | Success | Failed | Cancelled | Skipped | Unresolved | Success Rate | +|----------|-------|---------|--------|-----------|---------|------------|--------------| ${metrics.ci.workflows .map( (w) => @@ -580,7 +609,7 @@ ${metrics.ci.workflows - **Issue velocity:** ${metrics.issues.closedDuringPeriod} issues closed, ${metrics.issues.openedDuringPeriod} new issues opened - **PR velocity:** ${metrics.pullRequests.mergedDuringPeriod} PRs merged, ${metrics.pullRequests.openedDuringPeriod} new PRs opened -- **CI reliability:** ${formatSuccessRate(metrics.ci)} success rate across ${metrics.ci.totalRuns - metrics.ci.skippedRuns} executed workflow runs (${metrics.ci.skippedRuns} skipped, ${metrics.ci.failedRuns} failures) +- **CI reliability:** ${formatSuccessRate(metrics.ci)} success rate across ${metrics.ci.resolvedRuns} resolved workflow runs (${metrics.ci.skippedRuns} skipped, ${metrics.ci.otherRuns} unresolved, ${metrics.ci.failedRuns} failures) ---