diff --git a/.github/workflows/pr.yml b/.github/workflows/pr.yml index bb9bccdfa25..a1701bb96dc 100644 --- a/.github/workflows/pr.yml +++ b/.github/workflows/pr.yml @@ -692,20 +692,27 @@ jobs: "$chrome" --version echo "ORCA_MOBILE_WEB_RENDER_BROWSER=$chrome" >> "$GITHUB_ENV" + # The drawer check runs on WebKit as well as Chrome, because the shell's iOS WebView is + # WebKit and the Chrome above cannot stand in for it. Downloaded rather than resolved from + # the runner: Ubuntu ships no WebKit build to point at. + - name: Install WebKit for the drawer check + run: pnpm exec playwright install --with-deps webkit + - name: Build and verify the app bundle run: pnpm run build:mobile-web:app # The bundling tests skip themselves where mobile dependencies are absent, which is how they # stay green in the sharded `test` job. This is the job that installs them, so here a missing # install has to fail rather than skip everything the job exists to run. - - name: Builder, override census and render check + - name: Builder, override census and render checks env: ORCA_MOBILE_WEB_APP_DEPS_REQUIRED: '1' run: | pnpm exec vitest run --config config/vitest.config.ts \ config/scripts/build-mobile-web-app-bundle.test.mjs \ config/scripts/mobile-web-app-web-overrides.test.mjs \ - config/scripts/mobile-web-app-render.test.mjs + config/scripts/mobile-web-app-render.test.mjs \ + config/scripts/mobile-web-app-drawer-render.test.mjs cross-version-wire: name: cross-version wire compatibility diff --git a/config/scripts/mobile-web-app-drawer-render.test.mjs b/config/scripts/mobile-web-app-drawer-render.test.mjs new file mode 100644 index 00000000000..ae8530a25c6 --- /dev/null +++ b/config/scripts/mobile-web-app-drawer-render.test.mjs @@ -0,0 +1,272 @@ +import { mkdtemp, rm } from 'node:fs/promises' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterAll, beforeAll, describe, expect, it } from 'vitest' +import { chromium, webkit } from 'playwright-core' +import { buildMobileWebAppBundle } from './build-mobile-web-app-bundle.mjs' +import { mobileWebAppDependenciesPresent } from './mobile-web-app-bundle-dependencies.mjs' +import { + createBundleServer, + installShellDouble, + readBridgeFaultGrant, + readBridgeProtocolVersion, + readShellCsp +} from './mobile-web-app-render-harness.mjs' + +const HOST_ROUTE = '/h/render-check-host' +const SHELL_HOST = { + id: 'render-check-host', + name: 'Render Check Host', + endpoint: 'ws://render-check', + lastConnected: 1 +} + +const VIEWPORT = { width: 390, height: 844 } + +/** + * Both engines, because the defect this pins is not engine-specific. + * + * `useAnimatedStyle` without a dependency array registers a Reanimated mapper with no inputs + * (hook/useAnimatedStyle.js reads `updater.__closure`, which only the Babel plugin writes and + * esbuild never does). The mapper then runs once and never again, so the sheet keeps whichever + * translateY the first frame wrote. Chromium and WebKit both park it, so a Chromium-only pin + * would go green on an engine-specific theory that is not what is happening. + */ +const ENGINES = [ + { + name: 'chromium', + // CI runs this against the runner's Google Chrome rather than paying for a browser download, + // the same override shape as the render check next door. + launch: () => { + const executablePath = process.env.ORCA_MOBILE_WEB_RENDER_BROWSER + return chromium.launch({ + headless: true, + ...(executablePath ? { executablePath } : {}) + }) + } + }, + { name: 'webkit', launch: () => webkit.launch({ headless: true }) } +] + +const bundles = mobileWebAppDependenciesPresent() +const describeDrawer = bundles ? describe : describe.skip + +let scratch +let server +let origin +let cspHeader = null +let bridgeVersion = null +let faultGrant = null + +beforeAll(async () => { + if (!bundles) { + return + } + cspHeader = await readShellCsp() + bridgeVersion = await readBridgeProtocolVersion() + faultGrant = await readBridgeFaultGrant() + scratch = await mkdtemp(join(tmpdir(), 'orca-mobile-web-app-drawer-')) + const { outDir } = await buildMobileWebAppBundle({ outDir: join(scratch, 'bundle') }) + const served = await createBundleServer({ outDir, cspHeader }) + server = served.server + origin = served.origin +}, 180_000) + +afterAll(async () => { + server?.close() + if (scratch) { + await rm(scratch, { recursive: true, force: true }) + } +}) + +/** + * The sheet itself, by the name it gives itself. + * + * Not by its corner radius: that selected the sheet through a styling token, so a design change + * to the radius would have turned this pin into `sheet: false` -- a failure naming the wrong + * thing entirely. `testID` on the RN side renders as `data-testid` + * (react-native-web createDOMProps/index.js:832). + */ +function readDrawer() { + const handle = document.querySelector('[aria-label="Dismiss drawer"]') + if (!handle) { + return { open: false } + } + const sheet = document.querySelector('[data-testid="bottom-drawer-sheet"]') + if (!sheet) { + return { open: true, sheet: false } + } + const box = sheet.getBoundingClientRect() + return { + open: true, + sheet: true, + transform: getComputedStyle(sheet).transform, + top: Math.round(box.top), + bottom: Math.round(box.bottom), + height: Math.round(box.height) + } +} + +/** + * Installed at document start, so the counters cover the page's whole life rather than a window + * a poll happened to catch. Both are the page's own activity: `__raf` is every frame the page + * asked for, `__sheetWrites` every inline-style write Reanimated landed on the sheet. + */ +function instrumentFrames() { + globalThis.__raf = 0 + const realRaf = globalThis.requestAnimationFrame.bind(globalThis) + globalThis.requestAnimationFrame = (callback) => { + globalThis.__raf++ + return realRaf(callback) + } + globalThis.__sheetWrites = 0 + const observe = () => { + new MutationObserver((records) => { + for (const record of records) { + if (record.target.dataset?.testid === 'bottom-drawer-sheet') { + globalThis.__sheetWrites++ + } + } + }).observe(document.body, { subtree: true, attributes: true, attributeFilter: ['style'] }) + } + if (document.body) { + observe() + } else { + document.addEventListener('DOMContentLoaded', observe) + } +} + +/** The centre of the one leaf element whose whole text is `label`. */ +function centreOf(label) { + const leaf = [...document.querySelectorAll('*')].find( + (element) => element.childElementCount === 0 && element.textContent === label + ) + if (!leaf) { + return null + } + const box = leaf.getBoundingClientRect() + return { x: Math.round(box.x + box.width / 2), y: Math.round(box.y + box.height / 2) } +} + +describeDrawer('the bottom drawer on the page', () => { + for (const engine of ENGINES) { + it(`slides the sheet onto the screen in ${engine.name}`, async () => { + const browser = await engine.launch() + try { + // Motion on, stated rather than inherited. Under `prefers-reduced-motion: reduce` + // Reanimated finishes `withTiming` in one frame, so a mapper that only ever runs once + // still lands on the final translateY and this pin would pass on the broken build. + // Context-level and before navigation, both load-bearing: Reanimated latches the query + // into a module-level const at import (ReducedMotion.js:8-10), so an `emulateMedia` call + // after `goto` would leave the assertion below passing over an already-latched `true`. + const page = await browser.newPage({ + viewport: VIEWPORT, + reducedMotion: 'no-preference' + }) + const errors = [] + page.on('pageerror', (error) => errors.push(`${error.name}: ${error.message}`)) + await page.addInitScript(instrumentFrames) + await page.addInitScript(installShellDouble, { + version: bridgeVersion, + sessionId: 'render-check-session', + buildId: 'render-check-build', + route: { pathname: HOST_ROUTE }, + host: SHELL_HOST, + storage: {}, + faultGrant + }) + await page.goto(`${origin}/`, { waitUntil: 'load' }) + // The precondition the assertions below rest on, read off the page rather than assumed. + expect( + await page.evaluate(() => matchMedia('(prefers-reduced-motion: reduce)').matches) + ).toBe(false) + await page.waitForFunction( + () => document.documentElement.dataset.orcaWebEntry === 'mounted', + { timeout: 30_000, polling: 250 } + ) + // The filter sheet, not the row's action sheet: both are the same MountedBottomDrawer, and + // this one opens from the header, which needs nothing of the list's own layout. + const chip = await page.waitForFunction(centreOf, 'Filter', { + timeout: 30_000, + polling: 250 + }) + const at = await chip.jsonValue() + // Both counters start at the click, so what they measure is the enter animation's window + // and not everything the page did while it was booting. + await page.evaluate(() => { + globalThis.__rafAtClick = globalThis.__raf + globalThis.__sheetWrites = 0 + }) + await page.mouse.click(at.x, at.y) + const opened = await page + .waitForFunction( + () => { + const handle = document.querySelector('[aria-label="Dismiss drawer"]') + return handle ? true : null + }, + { timeout: 10_000, polling: 100 } + ) + .then(() => true) + expect(opened, errors.join(' | ')).toBe(true) + + // Wait for the animation to arrive rather than for a clock. A fixed pause makes the pin + // a race on a loaded runner: too short and a healthy-but-slow engine reads as parked, + // and the failure names the transform instead of the wait. A sheet that is genuinely + // parked never moves, so this times out and the assertions below still report what it + // found -- the same red, minus the timing assumption. + await page + .waitForFunction( + () => { + const sheet = document.querySelector('[data-testid="bottom-drawer-sheet"]') + return sheet && getComputedStyle(sheet).transform === 'matrix(1, 0, 0, 1, 0, 0)' + ? true + : null + }, + { timeout: 15_000, polling: 50 } + ) + .catch(() => null) + const drawer = await page.evaluate(readDrawer) + expect(drawer.sheet, JSON.stringify(drawer)).toBe(true) + + // The precondition, named, because the transform below cannot on its own tell a mapper + // that is not subscribed from an engine that never ran the animation at all. Both leave + // a parked sheet and only the first is this pin's subject. + // + // `requestAnimationFrame` is the one that separates them. `withTiming` drives itself by + // scheduling a frame per step (valueSetter.js `step`), and it does that whether or not + // any mapper is listening, so frames during this window mean the shared value moved. + // Sheet writes separate nothing and are carried as context only. The broken build writes + // once, an engine that never animated writes once, and -- measured under `--cpus=0.35` + // in Playwright's Linux image -- a healthy page starved of frames also reaches + // translateY(0) in a single write, because `withTiming` covers the whole 180ms in one + // step when that is all the frames it gets. Asserting on the count would red that page. + const frames = await page.evaluate(() => ({ + raf: globalThis.__raf - globalThis.__rafAtClick, + sheetWrites: globalThis.__sheetWrites + })) + expect( + frames.raf, + `${engine.name}: the page was given no animation frames after the sheet opened, so ` + + 'the enter animation never ran and the transform proves nothing about the mapper' + ).toBeGreaterThan(0) + + // Reanimated's own write, once its mapper has run to the end of `progress`. The initial + // inline style is a full viewport of translateY, so a mapper that stopped after its first + // frame leaves a matrix here with a large offset instead of none. + expect( + drawer.transform, + `${engine.name}: ${String(frames.sheetWrites)} style write(s) on the sheet across ` + + `${String(frames.raf)} frame(s) -- ${JSON.stringify(drawer)}` + ).toBe('matrix(1, 0, 0, 1, 0, 0)') + // And where that leaves the sheet: bottom-anchored inside the viewport, which is the + // thing the user sees and the thing a parked sheet gets wrong. + expect(drawer.bottom, JSON.stringify(drawer)).toBe(VIEWPORT.height) + expect(drawer.top, JSON.stringify(drawer)).toBeGreaterThan(0) + expect(errors).toEqual([]) + await page.close() + } finally { + await browser.close() + } + }, 120_000) + } +}) diff --git a/config/scripts/mobile-web-app-render-harness.mjs b/config/scripts/mobile-web-app-render-harness.mjs new file mode 100644 index 00000000000..b438fadd442 --- /dev/null +++ b/config/scripts/mobile-web-app-render-harness.mjs @@ -0,0 +1,224 @@ +import { createServer } from 'node:http' +import { readFile } from 'node:fs/promises' +import { join } from 'node:path' +import { fileURLToPath } from 'node:url' + +export const projectDir = fileURLToPath(new URL('../..', import.meta.url)) + +/** + * Both CSP constants are a list of quoted directives with `//` comments between them, and those + * comments quote directive text. Dropping comment lines first is what keeps a comment out of the + * header a test serves. + */ +export function parseCspDirectives(source, startMarker, endMarker) { + const start = source.indexOf(startMarker) + const end = source.indexOf(endMarker) + if (start === -1 || end < start) { + throw new Error(`could not find ${startMarker} .. ${endMarker}`) + } + const body = source + .slice(start, end) + .split('\n') + .filter((line) => !line.trimStart().startsWith('//')) + .join('\n') + const directives = [...body.matchAll(/"([^"]+)"/g)].map((match) => match[1]) + if (directives.length < 10) { + throw new Error('could not parse the shell CSP') + } + return directives.join('; ') +} + +/** + * The shipped policy, read from the Kotlin source so a test cannot drift from what the shell + * actually sends. Parsed rather than imported: the constant lives in a JVM module. + */ +export async function readShellCsp() { + const source = await readFile( + join( + projectDir, + 'mobile/modules/orca-mobile-web-shell/android/src/main/java/expo/modules/orcamobilewebshell/MobileWebShellCsp.kt' + ), + 'utf8' + ) + return parseCspDirectives(source, 'listOf(', ').joinToString') +} + +/** + * The envelope version the page speaks, read from the contract rather than written down twice. A + * bumped `v` would otherwise reach a test as a 30s timeout naming nothing. + */ +export async function readBridgeProtocolVersion() { + const source = await readFile( + join(projectDir, 'mobile/src/mobile-web-shell/bridge/bridge-envelope.ts'), + 'utf8' + ) + const match = /BRIDGE_PROTOCOL_VERSION = (\d+)/.exec(source) + if (!match) { + throw new Error('could not read BRIDGE_PROTOCOL_VERSION') + } + return Number(match[1]) +} + +/** The grant the shell offers every page, read from the same source for the same reason. */ +export async function readBridgeFaultGrant() { + const source = await readFile( + join(projectDir, 'mobile/src/mobile-web-shell/bridge/bridge-envelope.ts'), + 'utf8' + ) + const match = /BRIDGE_FAULT_GRANT = '([a-zA-Z]+)'/.exec(source) + if (!match) { + throw new Error('could not read BRIDGE_FAULT_GRANT') + } + return match[1] +} + +/** + * The shell's half of the bridge, as the page's channel sees it. + * + * The entry mounts nothing until `init` lands, so a render check with no shell renders no route at + * all. This answers `ready`, answers the methods `replies` names, and refuses everything else: a + * real reply would make this file the place domain behaviour is decided, and every screen below + * already has a state for an RPC that failed. `grants` and `pageRoutes` are what the shell would + * have negotiated, and every notify the page posts is kept whole in `__orcaRenderCheckNotifies`, + * because a control that handed something to the shell and one that did nothing look the same on + * the document. + * + * Serialized as a page init script, so it takes plain data and closes over nothing. + */ +export function installShellDouble({ + version, + sessionId, + buildId, + route, + host, + storage, + faultGrant, + grants, + pageRoutes = null, + replies +}) { + // Where the page's own fault reports land. Read back after the render, so a route that threw + // under the boundary names itself instead of timing out as a page that never mounted. + globalThis.__orcaRenderCheckFaults = [] + // Every grant-gated notify the page posted, whole and in order. A control that decided to hand + // something to the shell and a control that did nothing look identical on the document; this is + // the only thing that tells them apart. + globalThis.__orcaRenderCheckNotifies = [] + const channel = { + postMessage: (json) => { + const frame = JSON.parse(json) + const answer = (message) => { + // A microtask, not a task: the page posts `ready` while its script is still running, and + // this keeps the answer behind it without moving a timer the page's backoff reads. + queueMicrotask(() => { + channel.onmessage?.({ data: JSON.stringify(message) }) + }) + } + if (frame.type === 'ready') { + answer({ + v: version, + type: 'init', + sessionId, + buildId, + connection: { + state: 'connected', + reconnectAttempt: 0, + lastConnectedAt: 1, + lastInboundAt: 1, + generation: 0 + }, + grants: { + rpc: { maxPendingRequests: 64, maxSubscriptions: 32 }, + // The fault grant alone unless the caller named a set: every check needs that one, + // and a check that names none must not be handed an undefined list. + native: grants ?? [faultGrant] + }, + ...(pageRoutes === null ? {} : { pageRoutes }), + // Omitted for a shell too old to name one, which is the case the page has a panel for. + ...(route === null ? {} : { route }), + ...(host === null ? {} : { host }), + storage + }) + return + } + if (frame.type === 'notify') { + globalThis.__orcaRenderCheckNotifies.push(frame) + if (frame.name === faultGrant) { + globalThis.__orcaRenderCheckFaults.push(frame.error.message) + } + return + } + // The result the caller named for this method, carried in the envelope a real host uses. + // Anything unnamed still takes the refusal below, so a screen only ever sees data a test + // asked for. + if (frame.type === 'request' && replies && Object.hasOwn(replies, frame.method)) { + answer({ + v: version, + type: 'reply', + id: frame.id, + payload: { id: frame.id, ok: true, result: replies[frame.method] } + }) + return + } + if (frame.type === 'request' || frame.type === 'subscribe') { + answer({ + v: version, + type: 'error', + id: frame.id, + error: { + category: 'RenderCheckShellDouble', + message: 'the render check answers no RPC', + isRpcDeliveryUnknown: false + } + }) + } + }, + onmessage: null + } + globalThis.orcaBridge = channel +} + +/** + * The page server the render checks run against: the built bundle, under the shell's own policy. + * + * `transformChunk` is how a check poisons one route chunk without building a second bundle. + */ +export async function createBundleServer({ outDir, cspHeader, transformChunk }) { + const server = createServer((request, response) => { + const path = new URL(request.url, 'http://localhost').pathname + // A browser asks for this on its own and the shell's WebView never does. The bundle carries + // no icon, so a 404 would put a console error in every check that runs against a full Chrome + // -- which is what CI resolves -- and none against the bundled headless shell. + if (path === '/favicon.ico') { + response.writeHead(204) + response.end() + return + } + // A route path serves the entrypoint and the page routes client-side. A path naming a file + // has to come out of the bundle or 404, the same as the shell's manifest map: answering it + // with the document instead would hide a publicPath the script cannot fetch from. + const namesAFile = path.slice(path.lastIndexOf('/')).includes('.') + const file = namesAFile ? path.slice(1) : 'index.html' + readFile(join(outDir, file)).then( + (real) => { + const bytes = transformChunk ? transformChunk(path, real) : real + const headers = { + 'content-type': file.endsWith('.js') ? 'text/javascript' : 'text/html' + } + // The document carries the shell's real policy, so a directive the page violates fails + // here rather than on a phone. Assets carry none, exactly as the native handler does. + if (file === 'index.html' && cspHeader) { + headers['content-security-policy'] = cspHeader + } + response.writeHead(200, headers) + response.end(bytes) + }, + () => { + response.writeHead(404) + response.end() + } + ) + }) + await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) + return { server, origin: `http://127.0.0.1:${String(server.address().port)}` } +} diff --git a/config/scripts/mobile-web-app-render.test.mjs b/config/scripts/mobile-web-app-render.test.mjs index 3b60d9db47c..570d7beabd9 100644 --- a/config/scripts/mobile-web-app-render.test.mjs +++ b/config/scripts/mobile-web-app-render.test.mjs @@ -1,14 +1,19 @@ -import { createServer } from 'node:http' import { mkdtemp, readFile, rm } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' import { afterAll, beforeAll, describe, expect, it } from 'vitest' import { chromium } from 'playwright-core' -import { fileURLToPath } from 'node:url' import { buildMobileWebAppBundle } from './build-mobile-web-app-bundle.mjs' import { mobileWebAppDependenciesPresent } from './mobile-web-app-bundle-dependencies.mjs' - -const projectDir = fileURLToPath(new URL('../..', import.meta.url)) +import { + createBundleServer, + installShellDouble, + parseCspDirectives, + projectDir, + readBridgeFaultGrant, + readBridgeProtocolVersion, + readShellCsp +} from './mobile-web-app-render-harness.mjs' // Why a real browser: the route tree is handed to expo-router's own ExpoRoot through a synthesized // RequireContext. Nothing short of mounting it proves that object is the shape ExpoRoot reads. @@ -53,159 +58,6 @@ let faultGrant = null const poisonedChunks = new Set() const POISON_MESSAGE = 'render check poisoned this route chunk' -/** - * Both CSP constants are a list of quoted directives with `//` comments between them, and those - * comments quote directive text. Dropping comment lines first is what keeps a comment out of the - * header this test serves. - */ -export function parseCspDirectives(source, startMarker, endMarker) { - const start = source.indexOf(startMarker) - const end = source.indexOf(endMarker) - if (start === -1 || end < start) { - throw new Error(`could not find ${startMarker} .. ${endMarker}`) - } - const body = source - .slice(start, end) - .split('\n') - .filter((line) => !line.trimStart().startsWith('//')) - .join('\n') - const directives = [...body.matchAll(/"([^"]+)"/g)].map((match) => match[1]) - if (directives.length < 10) { - throw new Error('could not parse the shell CSP') - } - return directives.join('; ') -} - -/** - * The envelope version the page speaks, read from the contract rather than written down twice. A - * bumped `v` would otherwise reach this file as a 30s timeout naming nothing. - */ -async function readBridgeProtocolVersion() { - const source = await readFile( - join(projectDir, 'mobile/src/mobile-web-shell/bridge/bridge-envelope.ts'), - 'utf8' - ) - const match = /BRIDGE_PROTOCOL_VERSION = (\d+)/.exec(source) - if (!match) { - throw new Error('could not read BRIDGE_PROTOCOL_VERSION') - } - return Number(match[1]) -} - -/** The grant the shell offers every page, read from the same source for the same reason. */ -async function readBridgeFaultGrant() { - const source = await readFile( - join(projectDir, 'mobile/src/mobile-web-shell/bridge/bridge-envelope.ts'), - 'utf8' - ) - const match = /BRIDGE_FAULT_GRANT = '([a-zA-Z]+)'/.exec(source) - if (!match) { - throw new Error('could not read BRIDGE_FAULT_GRANT') - } - return match[1] -} - -/** - * The shell's half of the bridge, as the page's channel sees it. - * - * The entry mounts nothing until `init` lands, so a render check with no shell renders no route at - * all. This answers `ready` and refuses everything else: a real reply would make this file the - * place domain behaviour is decided, and every screen below already has a state for an RPC that - * failed. The one message that matters here is the one that lets the tree mount. - */ -function installShellDouble({ - version, - sessionId, - buildId, - route, - host, - storage, - faultGrant, - grants, - pageRoutes -}) { - // Where the page's own fault reports land. Read back after the render, so a route that threw - // under the boundary names itself instead of timing out as a page that never mounted. - globalThis.__orcaRenderCheckFaults = [] - // Every grant-gated notify the page posted, whole and in order. A control that decided to hand - // something to the shell and a control that did nothing look identical on the document; this is - // the only thing that tells them apart. - globalThis.__orcaRenderCheckNotifies = [] - const channel = { - postMessage: (json) => { - const frame = JSON.parse(json) - const answer = (message) => { - // A microtask, not a task: the page posts `ready` while its script is still running, and - // this keeps the answer behind it without moving a timer the page's backoff reads. - queueMicrotask(() => { - channel.onmessage?.({ data: JSON.stringify(message) }) - }) - } - if (frame.type === 'ready') { - answer({ - v: version, - type: 'init', - sessionId, - buildId, - connection: { - state: 'connected', - reconnectAttempt: 0, - lastConnectedAt: 1, - lastInboundAt: 1, - generation: 0 - }, - grants: { - rpc: { maxPendingRequests: 64, maxSubscriptions: 32 }, - native: grants - }, - ...(pageRoutes === null ? {} : { pageRoutes }), - // Omitted for a shell too old to name one, which is the case the page has a panel for. - ...(route === null ? {} : { route }), - ...(host === null ? {} : { host }), - storage - }) - return - } - if (frame.type === 'notify') { - globalThis.__orcaRenderCheckNotifies.push(frame) - if (frame.name === faultGrant) { - globalThis.__orcaRenderCheckFaults.push(frame.error.message) - } - return - } - if (frame.type === 'request' || frame.type === 'subscribe') { - answer({ - v: version, - type: 'error', - id: frame.id, - error: { - category: 'RenderCheckShellDouble', - message: 'the render check answers no RPC', - isRpcDeliveryUnknown: false - } - }) - } - }, - onmessage: null - } - globalThis.orcaBridge = channel -} - -/** - * The shipped policy, read from the Kotlin source so this test cannot drift from what the shell - * actually sends. Parsed rather than imported: the constant lives in a JVM module. - */ -async function readShellCsp() { - const source = await readFile( - join( - projectDir, - 'mobile/modules/orca-mobile-web-shell/android/src/main/java/expo/modules/orcamobilewebshell/MobileWebShellCsp.kt' - ), - 'utf8' - ) - return parseCspDirectives(source, 'listOf(', ').joinToString') -} - beforeAll(async () => { cspHeader = await readShellCsp() bridgeVersion = await readBridgeProtocolVersion() @@ -217,48 +69,19 @@ beforeAll(async () => { const built = await buildMobileWebAppBundle({ outDir: join(scratch, 'bundle') }) const { outDir } = built routeChunks = built.routeChunks - server = createServer((request, response) => { - const path = new URL(request.url, 'http://localhost').pathname - // A browser asks for this on its own and the shell's WebView never does. The bundle carries - // no icon, so a 404 would put a console error in every check that runs against a full Chrome - // -- which is what CI resolves -- and none against the bundled headless shell. - if (path === '/favicon.ico') { - response.writeHead(204) - response.end() - return - } - // A route path serves the entrypoint and the page routes client-side. A path naming a file - // has to come out of the bundle or 404, the same as the shell's manifest map: answering it - // with the document instead would hide a publicPath the script cannot fetch from. - const namesAFile = path.slice(path.lastIndexOf('/')).includes('.') - const file = namesAFile ? path.slice(1) : 'index.html' - readFile(join(outDir, file)).then( - (real) => { - // The real bytes with a throw in front: the module still links, so the importer resolves - // every export it asked for and then evaluation throws. A body replaced outright fails at - // link instead, which is a different failure from the one the boundary is here for. - const bytes = poisonedChunks.has(path) - ? `throw new Error(${JSON.stringify(POISON_MESSAGE)});\n${real.toString('utf8')}` - : real - const headers = { - 'content-type': file.endsWith('.js') ? 'text/javascript' : 'text/html' - } - // The document carries the shell's real policy, so a directive the page violates fails - // here rather than on a phone. Assets carry none, exactly as the native handler does. - if (file === 'index.html' && cspHeader) { - headers['content-security-policy'] = cspHeader - } - response.writeHead(200, headers) - response.end(bytes) - }, - () => { - response.writeHead(404) - response.end() - } - ) + // The real bytes with a throw in front: the module still links, so the importer resolves + // every export it asked for and then evaluation throws. A body replaced outright fails at + // link instead, which is a different failure from the one the boundary is here for. + const served = await createBundleServer({ + outDir, + cspHeader, + transformChunk: (path, real) => + poisonedChunks.has(path) + ? `throw new Error(${JSON.stringify(POISON_MESSAGE)});\n${real.toString('utf8')}` + : real }) - await new Promise((resolve) => server.listen(0, '127.0.0.1', resolve)) - origin = `http://127.0.0.1:${String(server.address().port)}` + server = served.server + origin = served.origin // CI runs this against the runner's Google Chrome rather than paying for a browser download, // the same reason and the same override shape as the orcad browser-provider job. const executablePath = process.env.ORCA_MOBILE_WEB_RENDER_BROWSER diff --git a/mobile/src/components/DragReorderList.tsx b/mobile/src/components/DragReorderList.tsx index 387686f0dab..b232434dba4 100644 --- a/mobile/src/components/DragReorderList.tsx +++ b/mobile/src/components/DragReorderList.tsx @@ -286,7 +286,7 @@ function DragReorderRow({ backgroundColor: colors.bgPanel, transform: [{ scale: 1 }] } - }) + }, [positions, activeKey, activeTop, rowKey, rowHeight]) return ( diff --git a/mobile/src/components/RightDrawer.tsx b/mobile/src/components/RightDrawer.tsx index eeeeccb0a2d..ba44822aac5 100644 --- a/mobile/src/components/RightDrawer.tsx +++ b/mobile/src/components/RightDrawer.tsx @@ -153,20 +153,23 @@ function MountedRightDrawer({ } }) - const drawerStyle = useAnimatedStyle(() => ({ - transform: [ - { - translateX: - interpolate(progress.value, [0, 1], [panelWidth, 0], Extrapolation.CLAMP) + - translateX.value - } - ] - })) + const drawerStyle = useAnimatedStyle( + () => ({ + transform: [ + { + translateX: + interpolate(progress.value, [0, 1], [panelWidth, 0], Extrapolation.CLAMP) + + translateX.value + } + ] + }), + [progress, translateX, panelWidth] + ) const backdropStyle = useAnimatedStyle(() => { const dragFade = interpolate(translateX.value, [0, panelWidth], [1, 0], Extrapolation.CLAMP) return { opacity: progress.value * dragFade } - }) + }, [progress, translateX, panelWidth]) return ( { const dragFade = interpolate(translateY.value, [0, 300], [1, 0], Extrapolation.CLAMP) return { opacity: progress.value * dragFade } - }) + }, [progress, translateY]) // Why: the sheet renders through a full-screen native window (its own Modal // below, or the shared BottomDrawerModalHost) so it always covers the viewport @@ -382,6 +382,8 @@ export function MountedBottomDrawer({ { + const path = join(directory, entry.name) + if (entry.isDirectory()) { + return entry.name === 'node_modules' ? [] : sourceFiles(path) + } + return sourceExtensions.has(extname(entry.name)) ? [path] : [] + }) +} + +/** Whether this `X.value` is being written rather than read. A write is an output, not an input. */ +function isWriteTarget(node: ts.PropertyAccessExpression): boolean { + const parent = node.parent + if (ts.isBinaryExpression(parent) && parent.left === node) { + // `=` through `??=`: every assignment operator sits in this one contiguous token range. + return ( + parent.operatorToken.kind >= ts.SyntaxKind.FirstAssignment && + parent.operatorToken.kind <= ts.SyntaxKind.LastAssignment + ) + } + if (ts.isPrefixUnaryExpression(parent)) { + // Only `++x` and `--x` mutate. `!x.value`, `-x.value`, `+x.value` and `~x.value` are reads, + // and taking every prefix operator for a write dropped those from the array's requirement. + return ( + parent.operator === ts.SyntaxKind.PlusPlusToken || + parent.operator === ts.SyntaxKind.MinusMinusToken + ) + } + // Postfix has no other operators: `x.value++` and `x.value--` are the whole set. + return ts.isPostfixUnaryExpression(parent) +} + +/** + * What each local name in this file means, for the hooks above, resolved through its imports. + * + * Matching on the callee's spelling would both miss and invent: `useAnimatedStyle as useAS` and + * `Reanimated.useAnimatedStyle` are the same hook under another name, and a local helper that + * happens to be called `useDerivedValue` is not this hook at all. Returns the local identifiers + * bound to each hook, plus the namespace names a member access has to go through. + */ +function reanimatedBindings(sourceFile: ts.SourceFile): { + byLocalName: Map + namespaces: Set +} { + const byLocalName = new Map() + const namespaces = new Set() + for (const statement of sourceFile.statements) { + if ( + !ts.isImportDeclaration(statement) || + !ts.isStringLiteral(statement.moduleSpecifier) || + statement.moduleSpecifier.text !== 'react-native-reanimated' + ) { + continue + } + const bindings = statement.importClause?.namedBindings + if (bindings && ts.isNamespaceImport(bindings)) { + namespaces.add(bindings.name.text) + } + if (bindings && ts.isNamedImports(bindings)) { + for (const element of bindings.elements) { + const imported = element.propertyName?.text ?? element.name.text + if (MAPPER_HOOKS.has(imported)) { + byLocalName.set(element.name.text, imported) + } + } + } + // The default export is the `Animated` namespace object, which carries no hooks. + } + return { byLocalName, namespaces } +} + +/** The hook this callee names, or null when it is not one of ours. */ +function resolveHook( + callee: ts.Expression, + bindings: ReturnType +): string | null { + if (ts.isIdentifier(callee)) { + return bindings.byLocalName.get(callee.text) ?? null + } + if ( + ts.isPropertyAccessExpression(callee) && + ts.isIdentifier(callee.expression) && + bindings.namespaces.has(callee.expression.text) && + MAPPER_HOOKS.has(callee.name.text) + ) { + return callee.name.text + } + return null +} + +/** Every `X` in an `X.value` read under this node, which is what the mapper has to listen to. */ +function sharedValuesRead(updater: ts.Node): Set { + const names = new Set() + const visit = (node: ts.Node): void => { + if ( + ts.isPropertyAccessExpression(node) && + node.name.text === 'value' && + ts.isIdentifier(node.expression) && + !isWriteTarget(node) + ) { + names.add(node.expression.text) + } + ts.forEachChild(node, visit) + } + visit(updater) + return names +} + +/** The identifiers a dependency array lists, ignoring entries that are not plain names. */ +function namesListed(dependencies: ts.ArrayLiteralExpression): Set { + return new Set(dependencies.elements.filter(ts.isIdentifier).map((element) => element.text)) +} + +/** + * Every mapper-hook call that was not handed a dependency array, or was handed one that leaves a + * shared value out. + * + * The second half is the one an array alone does not give: `inputs` becomes exactly the array + * (hook/useAnimatedStyle.js:338-341), so a value the updater reads but the array omits is a value + * the mapper never listens to. That updater then stops re-running when only that value changes, + * which is the same freeze as having no array at all, in one prop instead of all of them. + */ +function callsMissingDependencies(path: string, source: string, found: string[] = []): string[] { + const sourceFile = ts.createSourceFile( + path, + source, + ts.ScriptTarget.Latest, + true, + extname(path) === '.tsx' ? ts.ScriptKind.TSX : ts.ScriptKind.TS + ) + const bindings = reanimatedBindings(sourceFile) + const missing: string[] = [] + const visit = (node: ts.Node): void => { + if (ts.isCallExpression(node)) { + const name = resolveHook(node.expression, bindings) + const hook = name === null ? undefined : MAPPER_HOOKS.get(name) + if (name !== null && hook) { + found.push(name) + const dependencies = node.arguments[hook.dependencies] + const { line } = sourceFile.getLineAndCharacterOfPosition(node.getStart(sourceFile)) + const where = `${relative(mobileDirectory, path)}:${String(line + 1)} ${name}` + if (!dependencies) { + missing.push(where) + } else if (!ts.isArrayLiteralExpression(dependencies)) { + // An array built elsewhere counts as present: the hook only needs one to exist, and + // this file cannot see what a hoisted `const deps = [...]` holds. Completeness below + // therefore covers literal arrays only. + } else { + const listed = namesListed(dependencies) + const read = hook.updaters.flatMap((index) => { + const updater = node.arguments[index] + return updater ? [...sharedValuesRead(updater)] : [] + }) + for (const value of [...new Set(read)].sort()) { + if (!listed.has(value)) { + missing.push(`${where} omits ${value}`) + } + } + } + } + } + ts.forEachChild(node, visit) + } + visit(sourceFile) + return missing +} + +describe('reanimated mapper hooks in the web bundle', () => { + it('are all given a dependency array, because esbuild writes no worklet closure', () => { + const found: string[] = [] + const missing = scanned.flatMap((directory) => + sourceFiles(join(mobileDirectory, directory)).flatMap((path) => + path.endsWith('.test.ts') || path.endsWith('.test.tsx') + ? [] + : callsMissingDependencies(path, readFileSync(path, 'utf8'), found) + ) + ) + // The precondition the empty list above rests on. Binding resolution means a broken resolver + // reports nothing at all, which would read exactly like a clean tree. + expect(found.length).toBeGreaterThanOrEqual(5) + expect(missing).toEqual([]) + }) + + const FROM = "import { useAnimatedStyle, useAnimatedReaction } from 'react-native-reanimated'\n" + + it('finds a call with no dependency array, which is what makes the census above real', () => { + const found = callsMissingDependencies( + 'fixture.tsx', + `${FROM}const s = useAnimatedStyle(() => ({ opacity: progress.value }))\n` + ) + expect(found).toEqual(['fixture.tsx:2 useAnimatedStyle']) + }) + + it('reads useAnimatedReaction dependencies from its third argument, not its second', () => { + expect( + callsMissingDependencies( + 'fixture.tsx', + `${FROM}useAnimatedReaction(() => progress.value, (v) => { opacity.value = v })\n` + ) + ).toEqual(['fixture.tsx:2 useAnimatedReaction']) + expect( + callsMissingDependencies( + 'fixture.tsx', + `${FROM}useAnimatedReaction(() => progress.value, (v) => { opacity.value = v }, [progress])\n` + ) + ).toEqual([]) + }) + + it('names a shared value the updater reads but the array leaves out', () => { + const found = callsMissingDependencies( + 'fixture.tsx', + `${FROM}const s = useAnimatedStyle(() => ({ opacity: progress.value * fade.value }), [progress])\n` + ) + expect(found).toEqual(['fixture.tsx:2 useAnimatedStyle omits fade']) + }) + + it('does not ask for a value the updater only writes, which is an output', () => { + const found = callsMissingDependencies( + 'fixture.tsx', + `${FROM}useAnimatedReaction(() => progress.value, (v) => { opacity.value = v }, [progress])\n` + ) + expect(found).toEqual([]) + }) + + it('still asks for one that is read and written', () => { + const found = callsMissingDependencies( + 'fixture.tsx', + `${FROM}const s = useAnimatedStyle(() => { offset.value = offset.value + 1; return {} }, [])\n` + ) + expect(found).toEqual(['fixture.tsx:2 useAnimatedStyle omits offset']) + }) + + it('still asks for a value read under a negation, which is not a write', () => { + const found = callsMissingDependencies( + 'fixture.tsx', + `${FROM}const s = useAnimatedStyle(() => ({ opacity: !hidden.value ? 1 : 0 }), [])\n` + ) + expect(found).toEqual(['fixture.tsx:2 useAnimatedStyle omits hidden']) + }) + + it('and one read under a unary minus', () => { + const found = callsMissingDependencies( + 'fixture.tsx', + `${FROM}const s = useAnimatedStyle(() => ({ top: -offset.value }), [])\n` + ) + expect(found).toEqual(['fixture.tsx:2 useAnimatedStyle omits offset']) + }) + + it('does not ask for one that is only incremented', () => { + const found = callsMissingDependencies( + 'fixture.tsx', + `${FROM}const s = useAnimatedStyle(() => { count.value++; return {} }, [])\n` + ) + expect(found).toEqual([]) + }) + + it('accepts one that has a dependency array', () => { + const found = callsMissingDependencies( + 'fixture.tsx', + `${FROM}const s = useAnimatedStyle(() => ({ opacity: progress.value }), [progress])\n` + ) + expect(found).toEqual([]) + }) + + it('sees the hook through an alias, which spelling alone would miss', () => { + const found = callsMissingDependencies( + 'fixture.tsx', + "import { useAnimatedStyle as useAS } from 'react-native-reanimated'\n" + + 'const s = useAS(() => ({ opacity: progress.value }))\n' + ) + expect(found).toEqual(['fixture.tsx:2 useAnimatedStyle']) + }) + + it('sees it through a namespace import too', () => { + const found = callsMissingDependencies( + 'fixture.tsx', + "import * as Reanimated from 'react-native-reanimated'\n" + + 'const s = Reanimated.useAnimatedStyle(() => ({ opacity: progress.value }))\n' + ) + expect(found).toEqual(['fixture.tsx:2 useAnimatedStyle']) + }) + + it('leaves a local function of the same name alone', () => { + const found = callsMissingDependencies( + 'fixture.tsx', + 'function useDerivedValue(fn: () => number) { return fn() }\n' + + 'const v = useDerivedValue(() => progress.value)\n' + ) + expect(found).toEqual([]) + }) + + it('takes an array built elsewhere as present rather than missing', () => { + const found = callsMissingDependencies( + 'fixture.tsx', + `${FROM}const deps = [progress]\n` + + 'const s = useAnimatedStyle(() => ({ opacity: progress.value }), deps)\n' + ) + expect(found).toEqual([]) + }) +})