mirror of
https://github.com/stablyai/orca.git
synced 2026-09-21 16:02:20 +00:00
refactor(mobile): seam moves and the shared shell route guard for the source-control domain (OTA phase C, C4.1) (#21732)
* refactor(mobile): open PR sidebar URLs through the external-link seam (OTA phase C, C4.1) The three openers in the PR sidebar called `Linking.openURL` directly: a check's "open on the web", a comment's permalink, and a link inside comment Markdown. Inside the shell's WebView react-native-web routes that to `window.open(url, '_blank', 'noopener')`, which both shells refuse and which resolves anyway, so the tap reports success and opens nothing. Both C4 routes reach the sidebar, so both would have shipped that. The census is the point rather than the three edits. It derives the two future route closures through `mobileWebAppRouteClosure` and holds every module in them to the seam, so a module entering either closure later is ruled without anyone adding it here. Red first it named all three by `path:line`: CommentMarkdown.tsx:2, PRChecksSection.tsx:2, PRCommentCard.tsx:2, on both routes. The walk it runs was the third copy of one function, so it moves into the seam's own module beside the predicate that module exists to share, and the files and tasks censuses now call it too. It reports `path:line` where the copies reported paths; `reachesReactNativeLinking` keeps its name and its meaning and is now derived from the line list, so there is one rule. An empty offender list is empty in either shape, which is why repointing the two landed censuses moves nothing they assert. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): copy through the platform clipboard seam in review and conflicts (OTA phase C, C4.1) The two copy actions both C4 routes reach called `expo-clipboard` directly: the conflict section's refresh commands and the review sheet's notes. On the web that module is `navigator.clipboard`, which needs a secure context — the iOS shell serves the page from a custom scheme and Android from https, so the path works on one platform and silently not on the other. `useClipboardWriter` is the seam C2.4 landed for exactly that. Red first, the census named both routes: `ExpoClipboard.web.js` in each closure, and `src/platform/clipboard.web.ts` in neither. Both call sites also stopped ignoring whether the pasteboard took the text. The conflict section already returned on a throw, so the seam's rejection reaches an arm it had. `copyNotes` had none and its only caller is `void controller.copyNotes()`, so a rejection would have been unhandled with "Review notes copied" left on screen; it now catches and reports through the screen's own error line. That is the one behaviour change here and the reason `clipboard` joins its dependency array. Its suite mocked `setStringAsync` as resolving `undefined`, which the seam reads as a pasteboard that refused, so every copy would have gone down the new refusal arm unseen. The mock now resolves `true` and two cases pin both arms; mutating the catch away kills the refusal one. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): take the source-control router from the handoff seam (OTA phase C, C4.1) The hub takes its router once, in the openers hook, and passes it down to the runners and the panel — so one `useRouter()` is this domain's whole reach into routing, and it was expo-router's own. Inside the shell's page that posts no `navigate`, so the hub's push to review would stay in the document whatever its grants, and its push to a native route would paint Unmatched over the page. Inert today: no C4 route is registered yet. `use-mobile-source-control-runners.ts` is the second case and the reason the rule reads value imports rather than identifiers: it named expo-router only to write `ReturnType<typeof useRouter>`, a value import in a type position that keeps the module in the graph. `RouteHandoff` is the seam's own name for that type. The census is C3.1's, and its walk moves to `src/navigation` rather than being copied a second time; each domain keeps only its own evidence, the list of modules meant to hold a router. Red first it named both modules on the expo-router rule and reported no handoff caller at all. The C2.9 hop census is unchanged and cannot move: its targets come from the call sites, and the derivation over this tree returns the same ten targets and the same 26 unresolved sites before and after this commit, byte for byte. Its `HANDED_OFF` pin is over registered routes, of which this adds none. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * refactor(mobile): move the shell route guard out of the files domain (OTA phase C, C4.1) `src/files/mobile-file-shell-route.ts` was never about files: it parses a route against `BridgeInitRouteSchema` and builds the key a shell screen remounts on. It moves to `src/mobile-web-shell/shell-screen-route.ts` as `shellScreenRoute` / `shellScreenRouteKey`, with its test. The move is pure — with the rename applied and comments stripped, the old file and the new one diff to nothing. Three routes had grown their own copy of the call and two had none. The copies go: `agent-history` and `tasks` now ask the shared predicate, which is the same schema and the same fallback they already had. `index.tsx` had no guard at all, so a `.` or `..` host id was handed over and came back as "Update Orca to open this workspace" painted over the native list behind the switch; it now stays native. That is the one behaviour change here, pinned red first and killed by mutation. `web.tsx` keeps handing that route over on purpose and is exempt by name: its fallback is a redirect to the route the user came from, so the host's own verdict is the better answer there, which `mobile-web-shell-route.test.tsx` already pins. No `key=` expression moved; the three switches still key differently (host id, pathname, pathname plus params) and making them agree is a behaviour change for another PR. The census walks the route tree rather than a list, so a switch added later is held to both rules without being added here. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): mount the copy cases without a client instead of casting one (OTA phase C, C4.1) The changed-code gate flagged four type assertions on the two cases added with the clipboard seam: they stubbed an `RpcClient` the way the file's older cases do, and the gate reads changed lines. Copying reaches no client at all, so they mount without one, which is both cast-free and a truer statement of what the path needs. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): say what the censuses report and sort the red list by line (OTA phase C, C4.1 round 1) Round-1 folds, four wordings and one ordering. `externalLinkOffenders` said "every call site" and reports the line the name enters the module: a named import once, however many times the module calls `openURL`, because the import is what the rule is about and what has to go. Only a namespace import reports its uses, there being no single line to name. The docstring now says that. Its red list sorted the rendered strings, which puts `:10` before `:2`. It now sorts by path and then by line as a number. Pinned against a written fixture rather than the tree, because the case needs a module with sites either side of line ten and no module in a closure has to keep having one — the first fixture used lines 11 and 12, where both orders agree, and the mutation walked straight through it. `shell-screen-route.test.ts` still named the files screens in its describe after the guard stopped being theirs; it names what a switch does now. `router-seam-census.test-support.ts` excluded `.test-support.ts` from the walk, which the files census it was extracted from never did. Dropped, so both censuses walk the same set. Inert today: neither `src/files` nor `src/source-control` holds such a file, so it only decides the next one. The `web.tsx` exemption claimed a redirect "that looks like nothing happened". What was measured: adopting the guard there sends a `..` deep link through `Redirect href="/h/.."` to the host route, which this PR keeps native, so the developer lands on the host list with nothing said about why the page did not open. The route is `__DEV__`-only and the host's own failure screen is the better verdict. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * fix(mobile): see every react-native alias, normalise the host id, surface a refused copy (OTA phase C, C4.1 CodeRabbit) Three bots findings on #21732, all real. **Every alias, not the first.** `reactNativeLinkingSites` found namespace imports with `exec` and inspected only the first binding, so a module importing the namespace twice and calling `Linking.openURL` on the second reported no site at all. It reads every alias now and counts a line once however many meet on it. Red first with exactly that fixture. **The host id can be an array.** `app/h/[hostId]/index.tsx` read it bare, and Expo Router answers a repeated key with one: `String(['a','b'])` is `a,b`, `encodeURIComponent` makes that the single segment `a%2Cb`, and the segment rule accepts it — so the shell opened a page for a host nobody has. Through `firstParam`, as the other four switches do. Red first it handed over `/h/host-1%2Chost-2`, and the empty-array case found a second one: `[]` is truthy, so a bare read built `/h/` and handed that over too; `firstParam` answers `''` and the route stays native. That import pulls the source-control screen state, and with it the lucide barrel whose `LucideProvider` re-export is the gap the web build patches, so the suite mocks the barrel as the other suites do. It moves no page closure: the closure resolves `index.web.tsx`, which this does not touch, and the index route still measures 3426 modules, 289 local, 22 families. **A refused copy said nothing.** `PRConflictingFilesSection` caught the rejection and returned: no tick, no message, a tap indistinguishable from one that copied. The label now carries the third state, reusing the tasks page's own wording for it, and the component has its first test. Mutating the failure arm away reds it. Its prop narrows to `Pick<PRInfo, 'mergeable' | 'conflictSummary'>`, which is what it reads and what let the test drop a cast the gate flagged; every caller holds a full `PRInfo` and satisfies it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): read the censuses' subjects as code, not as text (OTA phase C, C4.1 round 2) Round-2 additions. A separate commit because `d4b14d54e5` was already made and this lane does not amend. **The seam walk parses now.** Matching `X.Linking` in the text named it inside a comment that talks about it and inside a string that quotes it, and the named-import regex did the same for a commented-out import. Checked against the previous implementation, all three fixtures were red there: the comment case reported lines 2 and 3, the string case reported the string's line beside the real call, and `// import { Linking } from 'react-native'` reported line 1. The walk builds a `SourceFile` and reads import declarations and property accesses, so comments and strings are gone by construction and the quote styles stop being a special case. Cost measured on the three closure censuses: 3.3 s, unchanged. **The route census reads the call, not the import.** A switch that keeps the import while the call goes — deleted, or moved behind a branch that never runs — looked exactly like one that asks. It now needs both, proved by mutation: dropping `shellScreenRoute(` from `tasks.tsx` while leaving its import names `tasks.tsx`. A fixture carries the same rule in isolation, since every switch in the tree calls what it imports and the case would otherwise be unfalsifiable against it. **And recognises a switch by its import** of `MobileWebShellScreen` rather than by `<MobileWebShellScreen` in the text, so an alias or a line break the formatter chose cannot hide one and a comment cannot invent one. The `app/h/[hostId]` root stays written out: deriving it from the manifest is not a one-liner from here, the manifest being an `.mjs` this test reads as text. What ties the two together instead is a new case asserting every registered pathname starts with that prefix, so a page route outside it fails rather than going unwalked. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): read the imported name, not the local one (OTA phase C, C4.1 CodeRabbit) `import { Linking as NativeLinking } from 'react-native'` went through the census untouched: the walk compared the specifier's local binding, which is `NativeLinking`, while the imported name lives in `propertyName` when a specifier renames it and only in `name` when it does not. Reproduced before the fix — the aliased import with a call beside it reported no site at all. Reading `propertyName ?? name` closes it in both directions. A module that renames `Linking` is named at its import line like any other, and a module that imports `View as Linking` is no longer named for a local binding that reaches nothing. The second was a false positive the old comparison had by construction. One more of the same class, found while checking and verified rather than assumed: `import RN from 'react-native'` typechecks in this project (tsc accepts it), and a default binding is the whole namespace exactly as `* as RN` is, so `RN.Linking.openURL` through it was invisible too. The default binding joins the alias set, which already reports uses rather than the import. Four fixtures. Three red on the previous walk: the renamed import, the local-only `Linking`, and the default import. The fourth — an alias imported that never reaches `Linking` — passed before and is here to hold the other half of the rule, that importing react-native is not itself the offence. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb * test(mobile): parse each module as its own kind, and read re-exports (OTA phase C, C4.1 CodeRabbit) Two ways a module reached `Linking` past the census, both reproduced before the change. Every file was parsed as TSX. In a `.ts` module `const id = <T>(value: T) => value` is a generic arrow; as TSX it is an unclosed JSX element, and the parser folds the rest of the file into the error node. A `RN.Linking.openURL` after one reported nothing, and so did the same call with its import above the arrow. The file name goes into the parse now and TypeScript reads the kind off the extension; `externalLinkOffenders` passes the real path, which it had all along. `ExportDeclaration` was never inspected, so `export { Linking } from 'react-native'` put the name back in reach of anything importing that module while the census saw an import list it was not on. All four shapes are read — named, renamed, `export *` and `export * as` — and reported at the export statement, which is the line to delete exactly as an import is. A re-export of another name, or of `Linking` from somewhere that is not react-native, stays unnamed. Seven fixtures. Five red on the previous walk: the `.ts` generic arrow and the four re-export shapes. The two that pass before and after hold the other half, that re-exporting is not itself the offence. The named-import and re-export clauses read `propertyName ?? name` through one helper rather than two spellings of it. Claude-Session: https://claude.ai/code/session_01JNnE9qzUZMMnqpZWCqM3nb --------- Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -4,23 +4,146 @@
|
||||
* Shared by every page route's census rather than restated in each: two spellings of one rule
|
||||
* drift, and the half that stops being enforced is the half nobody reads again.
|
||||
*/
|
||||
import { readFileSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
import ts from 'typescript-api'
|
||||
|
||||
/** The seam, as the web build resolves it: `.web.ts` wins under the builder's resolveExtensions,
|
||||
* and it is the one module a page closure may reach react-native's `Linking` from. */
|
||||
export const EXTERNAL_LINK_SEAM = 'src/platform/external-link.web.ts'
|
||||
|
||||
/**
|
||||
* Whether a module reaches react-native's own `Linking`, by name or through a namespace import.
|
||||
* Every line on which a module reaches react-native's own `Linking`, by name or through a
|
||||
* namespace import.
|
||||
*
|
||||
* Both quote styles: the tree is single-quoted by the formatter today, so a double-quoted
|
||||
* specifier would have walked past this unseen — and a census that cannot see a call site is one
|
||||
* that passes for the wrong reason.
|
||||
* Parsed rather than matched: a regex over the text names `Linking` inside a comment that talks
|
||||
* about it and inside a string that quotes it, and a census that reports a line nobody can act on
|
||||
* is one the next reader learns to ignore. The parser also settles the quote styles for free.
|
||||
*
|
||||
* A named import reports the import statement, once however many times the module calls through
|
||||
* it, because the import is the thing the rule is about and the thing that has to go. The imported
|
||||
* name is what counts, not the local one: `import { Linking as NativeLinking }` is the same import
|
||||
* spelled differently, and reading only the binding let it through.
|
||||
*
|
||||
* A namespace import reports its uses instead, there being no single line to name — `import * as RN
|
||||
* from 'react-native'` is not itself an offence — and every alias is read, because a module may
|
||||
* import the namespace twice and call on either. A default import is read the same way: this
|
||||
* project's interop settings accept `import RN from 'react-native'` (checked with tsc), so it is a
|
||||
* binding the whole namespace hangs off exactly as `* as RN` is.
|
||||
*
|
||||
* A re-export is reported at the export statement. `export { Linking } from 'react-native'` puts
|
||||
* the name back in reach of anything that imports this module, and so does `export *`, which
|
||||
* carries it along with everything else; the statement is the line to delete, exactly as an import
|
||||
* is.
|
||||
*
|
||||
* Lines rather than a boolean because a red census that names `path:line` is read once, and one
|
||||
* that names a file is grepped for. The boolean below is derived from this, so there is one rule.
|
||||
*
|
||||
* `fileName` decides the script kind, and the default is only for callers holding a source with no
|
||||
* path. In a `.ts` file `const id = <T>(value: T) => value` is a generic arrow; parsed as TSX it is
|
||||
* an unclosed JSX element, and everything after it — a later `RN.Linking.openURL` included — is
|
||||
* swallowed into the error node and never walked.
|
||||
*/
|
||||
export function reachesReactNativeLinking(source) {
|
||||
const named = /import\s*\{[^}]*\bLinking\b[^}]*\}\s*from\s*['"]react-native['"]/s
|
||||
const namespace = /import\s*\*\s*as\s*(\w+)\s*from\s*['"]react-native['"]/
|
||||
const asNamespace = namespace.exec(source)
|
||||
export function reactNativeLinkingSites(source, fileName = 'module.tsx') {
|
||||
// No explicit script kind: TypeScript reads it off the extension, which is the whole point.
|
||||
const parsed = ts.createSourceFile(fileName, source, ts.ScriptTarget.Latest, true)
|
||||
const lineOf = (node) => parsed.getLineAndCharacterOfPosition(node.getStart(parsed)).line + 1
|
||||
const sites = []
|
||||
const aliases = new Set()
|
||||
const fromReactNative = (statement) =>
|
||||
statement.moduleSpecifier !== undefined &&
|
||||
ts.isStringLiteral(statement.moduleSpecifier) &&
|
||||
statement.moduleSpecifier.text === 'react-native'
|
||||
/** `propertyName` is the exported/imported name when the clause renames it, `name` when it does not. */
|
||||
const namesLinking = (elements) =>
|
||||
elements.some((element) => (element.propertyName ?? element.name).text === 'Linking')
|
||||
for (const statement of parsed.statements) {
|
||||
if (ts.isExportDeclaration(statement) && fromReactNative(statement)) {
|
||||
const clause = statement.exportClause
|
||||
// No clause is `export *`, which carries `Linking` with everything else; a namespace export
|
||||
// hands the whole module over under one name. Both put it back in reach.
|
||||
if (clause === undefined || ts.isNamespaceExport(clause) || namesLinking(clause.elements)) {
|
||||
sites.push(lineOf(statement))
|
||||
}
|
||||
continue
|
||||
}
|
||||
if (!ts.isImportDeclaration(statement) || !fromReactNative(statement)) {
|
||||
continue
|
||||
}
|
||||
const clause = statement.importClause
|
||||
if (clause === undefined) {
|
||||
continue
|
||||
}
|
||||
if (clause.name !== undefined) {
|
||||
aliases.add(clause.name.text)
|
||||
}
|
||||
const bindings = clause.namedBindings
|
||||
if (bindings === undefined) {
|
||||
continue
|
||||
}
|
||||
if (ts.isNamespaceImport(bindings)) {
|
||||
aliases.add(bindings.name.text)
|
||||
continue
|
||||
}
|
||||
if (namesLinking(bindings.elements)) {
|
||||
sites.push(lineOf(statement))
|
||||
}
|
||||
}
|
||||
if (aliases.size > 0) {
|
||||
const visit = (node) => {
|
||||
if (
|
||||
ts.isPropertyAccessExpression(node) &&
|
||||
ts.isIdentifier(node.expression) &&
|
||||
aliases.has(node.expression.text) &&
|
||||
node.name.text === 'Linking'
|
||||
) {
|
||||
// The line, not the expression: two aliases meeting on one line are one site.
|
||||
sites.push(lineOf(node))
|
||||
}
|
||||
ts.forEachChild(node, visit)
|
||||
}
|
||||
ts.forEachChild(parsed, visit)
|
||||
}
|
||||
return [...new Set(sites)].sort((left, right) => left - right)
|
||||
}
|
||||
|
||||
/** Whether a module reaches react-native's own `Linking`. */
|
||||
export function reachesReactNativeLinking(source, fileName = 'module.tsx') {
|
||||
return reactNativeLinkingSites(source, fileName).length > 0
|
||||
}
|
||||
|
||||
/**
|
||||
* Every module in a route's closure that can reach a URL without the seam, as `path:line`.
|
||||
*
|
||||
* The line is where the name enters the module, not where it is used: a named import is reported
|
||||
* once however many times the module calls `Linking.openURL`, because the import is what the rule
|
||||
* is about and what has to go. Only a namespace import reports its uses, there being no single
|
||||
* line to name — `import * as RN from 'react-native'` is not itself an offence.
|
||||
*
|
||||
* Here rather than beside each census: three copies of this walk existed before the source-control
|
||||
* routes wanted a fourth, and the seam's own module is where the rule they share belongs. A file
|
||||
* the closure names but this checkout cannot read is not an offender — the closure reports paths
|
||||
* relative to `mobile/`, and one outside it is read by its caller, not guessed at here.
|
||||
*/
|
||||
export function externalLinkOffenders(mobileDir, closure) {
|
||||
return (
|
||||
named.test(source) || (asNamespace !== null && source.includes(`${asNamespace[1]}.Linking`))
|
||||
closure.local
|
||||
.filter((file) => file !== EXTERNAL_LINK_SEAM)
|
||||
.flatMap((file) => {
|
||||
let source
|
||||
try {
|
||||
source = readFileSync(join(mobileDir, file), 'utf8')
|
||||
} catch {
|
||||
return []
|
||||
}
|
||||
// The path, so the parser takes the script kind from the extension rather than assuming TSX.
|
||||
return reactNativeLinkingSites(source, file).map((line) => [file, line])
|
||||
})
|
||||
// By path, then by line as a number: sorting the rendered strings puts `:10` before `:2`, and
|
||||
// a red list is read top to bottom against the file it names.
|
||||
.sort(([leftFile, leftLine], [rightFile, rightLine]) =>
|
||||
leftFile === rightFile ? leftLine - rightLine : leftFile < rightFile ? -1 : 1
|
||||
)
|
||||
.map(([file, line]) => `${file}:${line}`)
|
||||
)
|
||||
}
|
||||
|
||||
@@ -1,5 +1,16 @@
|
||||
import { mkdtempSync, writeFileSync } from 'node:fs'
|
||||
import { tmpdir } from 'node:os'
|
||||
import { join } from 'node:path'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { reachesReactNativeLinking } from './mobile-web-app-external-link-seam.mjs'
|
||||
import {
|
||||
externalLinkOffenders,
|
||||
reachesReactNativeLinking,
|
||||
reactNativeLinkingSites
|
||||
} from './mobile-web-app-external-link-seam.mjs'
|
||||
|
||||
const mobileDir = fileURLToPath(new URL('../../mobile/', import.meta.url))
|
||||
|
||||
describe('the seam predicate', () => {
|
||||
it.each([
|
||||
["import { Linking } from 'react-native'", true],
|
||||
@@ -12,3 +23,176 @@ describe('the seam predicate', () => {
|
||||
expect(reachesReactNativeLinking(source)).toBe(expected)
|
||||
})
|
||||
})
|
||||
|
||||
describe('where the seam predicate says a module reaches Linking', () => {
|
||||
it('reports the import line, which is what a red census is read for', () => {
|
||||
expect(
|
||||
reactNativeLinkingSites(
|
||||
"import { View } from 'react-native'\n\nimport {\n Linking\n} from 'react-native'\n"
|
||||
)
|
||||
).toEqual([3])
|
||||
})
|
||||
|
||||
it('reports every line a namespace import is used on, not just the import', () => {
|
||||
expect(
|
||||
reactNativeLinkingSites(
|
||||
"import * as RN from 'react-native'\nRN.Linking.openURL(a)\nconst b = 1\nRN.Linking.openURL(c)\n"
|
||||
)
|
||||
).toEqual([2, 4])
|
||||
})
|
||||
|
||||
it('inspects every alias, not just the first namespace import', () => {
|
||||
// Two namespace imports of react-native, the first unused. Reading only the first alias makes
|
||||
// a module that calls `Linking.openURL` on the second report no site at all.
|
||||
expect(
|
||||
reactNativeLinkingSites(
|
||||
"import * as Unused from 'react-native'\nimport * as RN from 'react-native'\nRN.Linking.openURL(u)\n"
|
||||
)
|
||||
).toEqual([3])
|
||||
})
|
||||
|
||||
it('counts a line once when two aliases meet on it', () => {
|
||||
expect(
|
||||
reactNativeLinkingSites(
|
||||
"import * as A from 'react-native'\nimport * as B from 'react-native'\nA.Linking.openURL(B.Linking)\n"
|
||||
)
|
||||
).toEqual([3])
|
||||
})
|
||||
|
||||
it('names an import that renames Linking, which reading the binding alone missed', () => {
|
||||
// `import { Linking as NativeLinking }` is the same import spelled differently; the imported
|
||||
// name lives in `propertyName` when a specifier renames it, and only in `name` when it does
|
||||
// not. Reading `name` alone let `NativeLinking.openURL` through the census entirely.
|
||||
expect(
|
||||
reactNativeLinkingSites(
|
||||
"import { Linking as NativeLinking } from 'react-native'\nNativeLinking.openURL(u)\n"
|
||||
)
|
||||
).toEqual([1])
|
||||
})
|
||||
|
||||
it('leaves alone a local binding that is only spelled Linking', () => {
|
||||
// The other half of reading `propertyName`: this module imports `View`, so naming it would be
|
||||
// a red line with nothing to fix at the end of it.
|
||||
expect(
|
||||
reactNativeLinkingSites("import { View as Linking } from 'react-native'\nLinking.foo()\n")
|
||||
).toEqual([])
|
||||
})
|
||||
|
||||
it('reads a default import as the namespace it is, which the interop here allows', () => {
|
||||
// `import RN from 'react-native'` typechecks here, so it is a binding the whole namespace
|
||||
// hangs off and a call through it is as invisible to a named-import rule as an alias was.
|
||||
expect(
|
||||
reactNativeLinkingSites("import RN from 'react-native'\nRN.Linking.openURL(u)\n")
|
||||
).toEqual([2])
|
||||
})
|
||||
|
||||
it('leaves alone an alias that is imported and never reaches Linking', () => {
|
||||
// An import of react-native is not the offence; reaching `Linking` through it is.
|
||||
expect(reactNativeLinkingSites("import * as RN from 'react-native'\nRN.Platform.OS\n")).toEqual(
|
||||
[]
|
||||
)
|
||||
})
|
||||
|
||||
it('parses a .ts module as TypeScript, where a generic arrow is not an unclosed tag', () => {
|
||||
// `const id = <T>(value: T) => value` is a generic arrow in a `.ts` file and an unclosed JSX
|
||||
// element in a `.tsx` one. Parsed as TSX, everything after it falls into the error node, so
|
||||
// the call below was never walked and the module reported nothing at all.
|
||||
expect(
|
||||
reactNativeLinkingSites(
|
||||
"import * as RN from 'react-native'\nconst id = <T>(value: T) => value\nRN.Linking.openURL(u)\n",
|
||||
'module.ts'
|
||||
)
|
||||
).toEqual([3])
|
||||
})
|
||||
|
||||
it.each([
|
||||
["export { Linking } from 'react-native'\n", 'a named re-export'],
|
||||
["export { Linking as L } from 'react-native'\n", 'a renamed re-export'],
|
||||
["export * from 'react-native'\n", 'a wildcard re-export, which carries it with the rest'],
|
||||
["export * as RN from 'react-native'\n", 'a namespace re-export']
|
||||
])('names %# : %s', (source) => {
|
||||
// A re-export puts `Linking` back in reach of whatever imports this module, so the route's
|
||||
// closure reaches it through a file that never imported it. The export statement is the line
|
||||
// to delete, exactly as an import is.
|
||||
expect(reactNativeLinkingSites(source, 'module.ts')).toEqual([1])
|
||||
})
|
||||
|
||||
it.each([
|
||||
["export { View } from 'react-native'\n", 're-exports something else'],
|
||||
[
|
||||
"export { Linking } from './local'\n",
|
||||
're-exports the name from somewhere that is not react-native'
|
||||
]
|
||||
])('leaves alone a module that %# : %s', (source) => {
|
||||
expect(reactNativeLinkingSites(source, 'module.ts')).toEqual([])
|
||||
})
|
||||
|
||||
it('ignores the name inside a comment, which text matching cannot', () => {
|
||||
// A module that talks about the rule is not breaking it, and a census that names a comment is
|
||||
// one whose red list the next reader learns to skip.
|
||||
expect(
|
||||
reactNativeLinkingSites(
|
||||
"import * as RN from 'react-native'\n// never call RN.Linking.openURL here\n/* nor RN.Linking */\n"
|
||||
)
|
||||
).toEqual([])
|
||||
})
|
||||
|
||||
it('ignores the name inside a string, and still sees the call beside it', () => {
|
||||
expect(
|
||||
reactNativeLinkingSites(
|
||||
"import * as RN from 'react-native'\nconst hint = 'use RN.Linking.openURL'\nRN.Linking.openURL(u)\n"
|
||||
)
|
||||
).toEqual([3])
|
||||
})
|
||||
|
||||
it('ignores a commented-out named import, which was the same class of miss', () => {
|
||||
expect(reactNativeLinkingSites("// import { Linking } from 'react-native'\n")).toEqual([])
|
||||
})
|
||||
|
||||
it('finds nothing in a module that only names the seam', () => {
|
||||
expect(
|
||||
reactNativeLinkingSites("import { openExternalLink } from '../platform/external-link'")
|
||||
).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe('the offenders in a closure', () => {
|
||||
// The seam itself imports `Linking` and is the one module allowed to, so a walk that did not
|
||||
// exempt it would report every closure as an offender and never be able to go green.
|
||||
const closure = { local: ['src/platform/external-link.web.ts', 'src/platform/external-link.ts'] }
|
||||
|
||||
it('exempts the seam and names the module that went around it', () => {
|
||||
expect(externalLinkOffenders(mobileDir, closure)).toEqual(['src/platform/external-link.ts:1'])
|
||||
})
|
||||
|
||||
it('ignores a path this checkout cannot read rather than calling it an offender', () => {
|
||||
expect(externalLinkOffenders(mobileDir, { local: ['src/not/a/file.ts'] })).toEqual([])
|
||||
})
|
||||
})
|
||||
|
||||
describe('the order a red list is read in', () => {
|
||||
// Against a written fixture rather than the tree: the ordering this pins needs one module with
|
||||
// sites on lines 2 and 10, the pair that sorts one way as numbers and the other as text, and no
|
||||
// module in the closure has to keep having one.
|
||||
const root = mkdtempSync(join(tmpdir(), 'orca-seam-census-'))
|
||||
const lines = ["import * as RN from 'react-native'", 'RN.Linking.openURL(a)']
|
||||
while (lines.length < 9) {
|
||||
lines.push('')
|
||||
}
|
||||
lines.push('RN.Linking.openURL(b)')
|
||||
writeFileSync(join(root, 'wide.ts'), `${lines.join('\n')}\n`)
|
||||
writeFileSync(join(root, 'above.ts'), "import { Linking } from 'react-native'\n")
|
||||
|
||||
it('puts line 2 before line 10, which sorting the rendered strings does not', () => {
|
||||
// `:10` sorts before `:2` as text. The namespace import on line 1 is not itself an offence.
|
||||
expect(externalLinkOffenders(root, { local: ['wide.ts'] })).toEqual(['wide.ts:2', 'wide.ts:10'])
|
||||
})
|
||||
|
||||
it('orders by path first, so two modules never interleave', () => {
|
||||
expect(externalLinkOffenders(root, { local: ['wide.ts', 'above.ts'] })).toEqual([
|
||||
'above.ts:1',
|
||||
'wide.ts:2',
|
||||
'wide.ts:10'
|
||||
])
|
||||
})
|
||||
})
|
||||
|
||||
@@ -12,8 +12,6 @@
|
||||
* page route reaches and which the worktree list declares nothing for — and declares the grant
|
||||
* because its rows push to the preview in-page, under the session the explorer opened.
|
||||
*/
|
||||
import { readFileSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { mobileWebAppRouteClosure } from './build-mobile-web-app-bundle.mjs'
|
||||
@@ -21,7 +19,7 @@ import { MOBILE_WEB_PAGE_ROUTES } from './mobile-web-page-routes.mjs'
|
||||
import { mobileWebAppDependenciesPresent } from './mobile-web-app-bundle-dependencies.mjs'
|
||||
import {
|
||||
EXTERNAL_LINK_SEAM as SEAM,
|
||||
reachesReactNativeLinking
|
||||
externalLinkOffenders
|
||||
} from './mobile-web-app-external-link-seam.mjs'
|
||||
|
||||
const mobileDir = fileURLToPath(new URL('../../mobile/', import.meta.url))
|
||||
@@ -33,25 +31,12 @@ const PREVIEW = 'app/h/[hostId]/files/preview/[worktreeId].tsx'
|
||||
/** The seam's only in-domain consumer, and the reason the preview declares the grant itself. */
|
||||
const MARKDOWN = 'src/components/MobileMarkdown.tsx'
|
||||
|
||||
function offenders(closure) {
|
||||
return closure.local
|
||||
.filter((file) => file !== SEAM)
|
||||
.filter((file) => {
|
||||
try {
|
||||
return reachesReactNativeLinking(readFileSync(join(mobileDir, file), 'utf8'))
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
})
|
||||
.sort()
|
||||
}
|
||||
|
||||
describeClosure(
|
||||
'the files page closures',
|
||||
() => {
|
||||
it.each([EXPLORER, PREVIEW])('opens every external URL through the seam: %s', async (route) => {
|
||||
const closure = await mobileWebAppRouteClosure(route)
|
||||
expect(offenders(closure)).toEqual([])
|
||||
expect(externalLinkOffenders(mobileDir, closure)).toEqual([])
|
||||
})
|
||||
|
||||
it.each([EXPLORER, PREVIEW])(
|
||||
|
||||
@@ -0,0 +1,101 @@
|
||||
/**
|
||||
* What the source-control hub and the diff review page may reach for a URL.
|
||||
*
|
||||
* Inside the shell's WebView react-native-web's `Linking.openURL` calls
|
||||
* `window.open(url, '_blank', 'noopener')`, which both shells refuse — iOS returns nil from
|
||||
* `createWebViewWith`, Android false from `onCreateWindow` — and resolves whether or not anything
|
||||
* opened. A call site left on that path reports success into a tap that did nothing, which is the
|
||||
* one failure the `externalLink` grant exists to remove.
|
||||
*
|
||||
* Both routes reach the PR sidebar, and the sidebar is where this domain's openers are: a check's
|
||||
* "open on the web", a comment's permalink, and a link inside comment Markdown. So both are held
|
||||
* to the same rule and neither inherits it from the other.
|
||||
*
|
||||
* The rule, not the three call sites it happens to have today: a module entering either closure
|
||||
* later is held to it without anyone remembering to add it here.
|
||||
*/
|
||||
import { fileURLToPath } from 'node:url'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { mobileWebAppRouteClosure } from './build-mobile-web-app-bundle.mjs'
|
||||
import { mobileWebAppDependenciesPresent } from './mobile-web-app-bundle-dependencies.mjs'
|
||||
import {
|
||||
EXTERNAL_LINK_SEAM as SEAM,
|
||||
externalLinkOffenders
|
||||
} from './mobile-web-app-external-link-seam.mjs'
|
||||
|
||||
const mobileDir = fileURLToPath(new URL('../../mobile/', import.meta.url))
|
||||
const describeClosure = mobileWebAppDependenciesPresent() ? describe : describe.skip
|
||||
|
||||
const HUB = 'app/h/[hostId]/source-control/[worktreeId].tsx'
|
||||
const REVIEW = 'app/h/[hostId]/review/[worktreeId].tsx'
|
||||
|
||||
/** The sidebar both routes render, and the reason each declares the grant on its own account. */
|
||||
const PR_COMMENT_CARD = 'src/components/pr-sidebar/PRCommentCard.tsx'
|
||||
|
||||
/** The clipboard seam, as the web build resolves it. */
|
||||
const CLIPBOARD_SEAM = 'src/platform/clipboard.web.ts'
|
||||
|
||||
describeClosure(
|
||||
'the source-control and review page closures',
|
||||
() => {
|
||||
it.each([HUB, REVIEW])('opens every external URL through the seam: %s', async (route) => {
|
||||
const closure = await mobileWebAppRouteClosure(route)
|
||||
expect(externalLinkOffenders(mobileDir, closure)).toEqual([])
|
||||
})
|
||||
|
||||
it.each([HUB, REVIEW])('contains the seam, so the rule is not vacuous: %s', async (route) => {
|
||||
// Without this an empty offender list would also be what a closure reaching no link code at
|
||||
// all produces, and the census would pass against a page that opens nothing.
|
||||
const closure = await mobileWebAppRouteClosure(route)
|
||||
expect(closure.local).toContain(SEAM)
|
||||
expect(closure.local.length).toBeGreaterThan(400)
|
||||
})
|
||||
|
||||
it('reaches the openers from the PR sidebar, which both routes render', async () => {
|
||||
// The reason the grant is each route's own rather than one inherited through a hop: without
|
||||
// this, `externalLink` on both would be a line in a manifest nothing holds to a caller.
|
||||
const [hub, review] = await Promise.all([
|
||||
mobileWebAppRouteClosure(HUB),
|
||||
mobileWebAppRouteClosure(REVIEW)
|
||||
])
|
||||
expect(hub.local).toContain(PR_COMMENT_CARD)
|
||||
expect(review.local).toContain(PR_COMMENT_CARD)
|
||||
})
|
||||
},
|
||||
240_000
|
||||
)
|
||||
|
||||
/**
|
||||
* Neither route writes the clipboard through the browser's own.
|
||||
*
|
||||
* `expo-clipboard` resolves to `ExpoClipboard.web.js`, which is `navigator.clipboard`: it needs a
|
||||
* secure context, and the iOS shell serves the page from a custom scheme while Android serves
|
||||
* `https`, so that path works on one platform and silently not on the other. Both routes copy —
|
||||
* the conflict section's refresh commands, and the review sheet's notes — so both are granted
|
||||
* `native.clipboard.write` and both must reach it through the seam.
|
||||
*
|
||||
* Asserted as the module's absence from the closure rather than as a count of importers: a new
|
||||
* import anywhere in the tree puts the file back, whoever writes it and whatever they name it.
|
||||
*/
|
||||
describeClosure(
|
||||
'the clipboard the source-control and review pages reach',
|
||||
() => {
|
||||
it.each([HUB, REVIEW])(
|
||||
"does not carry expo-clipboard's web module at all: %s",
|
||||
async (route) => {
|
||||
const closure = await mobileWebAppRouteClosure(route)
|
||||
expect(closure.modules.filter((file) => file.endsWith('ExpoClipboard.web.js'))).toEqual([])
|
||||
}
|
||||
)
|
||||
|
||||
it.each([HUB, REVIEW])(
|
||||
'carries the seam that replaced it, so the absence above is not vacuous: %s',
|
||||
async (route) => {
|
||||
// An empty list is also what a closure reaching no clipboard code at all would produce.
|
||||
const closure = await mobileWebAppRouteClosure(route)
|
||||
expect(closure.local).toContain(CLIPBOARD_SEAM)
|
||||
}
|
||||
)
|
||||
},
|
||||
240_000
|
||||
)
|
||||
@@ -11,15 +11,13 @@
|
||||
* The rule, not the twelve call sites it happens to have today: a module entering this closure
|
||||
* later is held to it without anyone remembering to add it here.
|
||||
*/
|
||||
import { readFileSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
import { fileURLToPath } from 'node:url'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { mobileWebAppRouteClosure } from './build-mobile-web-app-bundle.mjs'
|
||||
import { mobileWebAppDependenciesPresent } from './mobile-web-app-bundle-dependencies.mjs'
|
||||
import {
|
||||
EXTERNAL_LINK_SEAM as SEAM,
|
||||
reachesReactNativeLinking
|
||||
externalLinkOffenders
|
||||
} from './mobile-web-app-external-link-seam.mjs'
|
||||
|
||||
const mobileDir = fileURLToPath(new URL('../../mobile/', import.meta.url))
|
||||
@@ -30,18 +28,9 @@ describeClosure(
|
||||
() => {
|
||||
it('opens every external URL through the platform seam', async () => {
|
||||
const closure = await mobileWebAppRouteClosure('app/h/[hostId]/tasks.tsx')
|
||||
const offenders = closure.local
|
||||
.filter((file) => file !== SEAM)
|
||||
.filter((file) => {
|
||||
try {
|
||||
// Which module the name comes from, not which text a call site writes: the tasks tree
|
||||
// still calls `Linking.openURL`, and that `Linking` is the barrel's seam-backed export.
|
||||
return reachesReactNativeLinking(readFileSync(join(mobileDir, file), 'utf8'))
|
||||
} catch {
|
||||
return false
|
||||
}
|
||||
})
|
||||
expect(offenders.sort()).toEqual([])
|
||||
// Which module the name comes from, not which text a call site writes: the tasks tree
|
||||
// still calls `Linking.openURL`, and that `Linking` is the barrel's seam-backed export.
|
||||
expect(externalLinkOffenders(mobileDir, closure)).toEqual([])
|
||||
})
|
||||
|
||||
it('contains the seam, so the rule above is not vacuous', async () => {
|
||||
|
||||
@@ -1,7 +1,7 @@
|
||||
import { useLocalSearchParams } from 'expo-router'
|
||||
import { BridgeInitRouteSchema } from '../../../../src/mobile-web-shell/bridge/bridge-envelope'
|
||||
import { MobileAgentSessionHistoryPanel } from '../../../../src/agent-history/MobileAgentSessionHistoryPanel'
|
||||
import { MobileWebShellScreen } from '../../../../src/mobile-web-shell/MobileWebShellScreen'
|
||||
import { shellScreenRoute } from '../../../../src/mobile-web-shell/shell-screen-route'
|
||||
import { useMobileWebShellEnabled } from '../../../../src/mobile-web-shell/use-mobile-web-shell-enabled'
|
||||
import { firstParam } from '../../../../src/source-control/mobile-source-control-screen-state'
|
||||
|
||||
@@ -26,8 +26,8 @@ import { firstParam } from '../../../../src/source-control/mobile-source-control
|
||||
*
|
||||
* The schema is the predicate rather than a copy of its bounds: two spellings of one rule drift,
|
||||
* and the half that matters is the half the page reads. C3.1 made the same call for the files
|
||||
* routes in `mobile-file-shell-route.ts`; once both are on main the two belong in one module
|
||||
* beside the schema, which is a contract file the C2 lane owns today.
|
||||
* routes first, and every switch now asks the one module beside the schema
|
||||
* rather than carrying its own copy of the call.
|
||||
*/
|
||||
export default function MobileAgentSessionHistoryScreen() {
|
||||
const params = useLocalSearchParams<{
|
||||
@@ -46,13 +46,13 @@ export default function MobileAgentSessionHistoryScreen() {
|
||||
if (enabled !== true || !hostId || !worktreeId) {
|
||||
return panel
|
||||
}
|
||||
const route = {
|
||||
const route = shellScreenRoute({
|
||||
pathname: `/h/${encodeURIComponent(hostId)}/agent-history/${encodeURIComponent(worktreeId)}`,
|
||||
// Omitted rather than empty: the page reads the label off the search half, and a `name=`
|
||||
// with nothing after it is a label, where an absent one lets the panel derive its own.
|
||||
...(name === '' ? {} : { params: { name } })
|
||||
}
|
||||
if (!BridgeInitRouteSchema.safeParse(route).success) {
|
||||
})
|
||||
if (route === null) {
|
||||
return panel
|
||||
}
|
||||
// Keyed on the route: a host captures the grants its session was opened with, so a screen
|
||||
|
||||
@@ -2,9 +2,9 @@ import { useLocalSearchParams } from 'expo-router'
|
||||
import { MobileFileExplorerPanel } from '../../../../src/files/MobileFileExplorerPanel'
|
||||
import { firstParam } from '../../../../src/source-control/mobile-source-control-screen-state'
|
||||
import {
|
||||
mobileFileShellRoute,
|
||||
mobileFileShellRouteKey
|
||||
} from '../../../../src/files/mobile-file-shell-route'
|
||||
shellScreenRoute,
|
||||
shellScreenRouteKey
|
||||
} from '../../../../src/mobile-web-shell/shell-screen-route'
|
||||
import { MobileWebShellScreen } from '../../../../src/mobile-web-shell/MobileWebShellScreen'
|
||||
import { useMobileWebShellEnabled } from '../../../../src/mobile-web-shell/use-mobile-web-shell-enabled'
|
||||
|
||||
@@ -39,7 +39,7 @@ export default function MobileFileExplorerScreen() {
|
||||
|
||||
const route =
|
||||
hostId && worktreeId
|
||||
? mobileFileShellRoute({
|
||||
? shellScreenRoute({
|
||||
pathname: `/h/${encodeURIComponent(hostId)}/files/${encodeURIComponent(worktreeId)}`,
|
||||
// Omitted rather than empty: the panel derives its own label from the worktree id when
|
||||
// the caller named none, where `name=` with nothing after it is a label.
|
||||
@@ -55,7 +55,7 @@ export default function MobileFileExplorerScreen() {
|
||||
// left. The key is what makes the change a remount, which disposes that bridge in the commit.
|
||||
return (
|
||||
<MobileWebShellScreen
|
||||
key={mobileFileShellRouteKey(route)}
|
||||
key={shellScreenRouteKey(route)}
|
||||
hostId={hostId}
|
||||
route={route}
|
||||
fallback={native}
|
||||
|
||||
@@ -5,9 +5,9 @@ import {
|
||||
normalizeMobileFilePreviewRouteParams
|
||||
} from '../../../../../src/files/mobile-file-preview-route'
|
||||
import {
|
||||
mobileFileShellRoute,
|
||||
mobileFileShellRouteKey
|
||||
} from '../../../../../src/files/mobile-file-shell-route'
|
||||
shellScreenRoute,
|
||||
shellScreenRouteKey
|
||||
} from '../../../../../src/mobile-web-shell/shell-screen-route'
|
||||
import { MobileWebShellScreen } from '../../../../../src/mobile-web-shell/MobileWebShellScreen'
|
||||
import { useMobileWebShellEnabled } from '../../../../../src/mobile-web-shell/use-mobile-web-shell-enabled'
|
||||
|
||||
@@ -45,7 +45,7 @@ export default function MobileFilePreviewRoute() {
|
||||
const native = <MobileFilePreviewScreen route={route} />
|
||||
|
||||
const shellRoute = route.ok
|
||||
? mobileFileShellRoute({
|
||||
? shellScreenRoute({
|
||||
pathname: `/h/${encodeURIComponent(route.params.hostId)}/files/preview/${encodeURIComponent(
|
||||
route.params.worktreeId
|
||||
)}`,
|
||||
@@ -63,7 +63,7 @@ export default function MobileFilePreviewRoute() {
|
||||
// was opened on, with nothing to tell it otherwise.
|
||||
return (
|
||||
<MobileWebShellScreen
|
||||
key={mobileFileShellRouteKey(shellRoute)}
|
||||
key={shellScreenRouteKey(shellRoute)}
|
||||
hostId={route.params.hostId}
|
||||
route={shellRoute}
|
||||
fallback={native}
|
||||
|
||||
@@ -1,8 +1,10 @@
|
||||
import { useLocalSearchParams } from 'expo-router'
|
||||
import { WorkspaceDetailPlaceholder } from '../../../src/components/WorkspaceDetailPlaceholder'
|
||||
import { firstParam } from '../../../src/source-control/mobile-source-control-screen-state'
|
||||
import { HostScreen } from '../../../src/host-screen/HostScreen'
|
||||
import { useResponsiveLayout } from '../../../src/layout/responsive-layout'
|
||||
import { MobileWebShellScreen } from '../../../src/mobile-web-shell/MobileWebShellScreen'
|
||||
import { shellScreenRoute } from '../../../src/mobile-web-shell/shell-screen-route'
|
||||
import { useMobileWebShellEnabled } from '../../../src/mobile-web-shell/use-mobile-web-shell-enabled'
|
||||
|
||||
/**
|
||||
@@ -21,10 +23,21 @@ import { useMobileWebShellEnabled } from '../../../src/mobile-web-shell/use-mobi
|
||||
* screen rather than the native list this route already has.
|
||||
*/
|
||||
function HostListScreen() {
|
||||
const { hostId } = useLocalSearchParams<{ hostId: string }>()
|
||||
// Through `firstParam`, as the other four switches do: expo-router answers a repeated key with
|
||||
// an array, and a bare read puts it straight into the template, where `String(['a','b'])` is
|
||||
// `a,b` and `encodeURIComponent` makes it the single segment `a%2Cb` — which the bridge's
|
||||
// segment rule accepts, so the shell would open a page for a host nobody has. An empty array is
|
||||
// truthy, so a bare read also builds `/h/` and hands that over; this answers `''` and stays.
|
||||
const params = useLocalSearchParams<{ hostId?: string | string[] }>()
|
||||
const hostId = firstParam(params.hostId)
|
||||
const enabled = useMobileWebShellEnabled()
|
||||
|
||||
if (enabled !== true || !hostId) {
|
||||
// Asked here as every switch asks it: encoding does not save a `.` or `..` host id, which fails
|
||||
// the bridge's segment rule, and handing that over paints the page's failure screen over the
|
||||
// native list this route already has.
|
||||
const route = shellScreenRoute({ pathname: `/h/${encodeURIComponent(hostId)}` })
|
||||
|
||||
if (enabled !== true || !hostId || route === null) {
|
||||
return <HostScreen />
|
||||
}
|
||||
return (
|
||||
@@ -33,7 +46,7 @@ function HostListScreen() {
|
||||
// so a host id change must be a remount rather than a prop update.
|
||||
key={hostId}
|
||||
hostId={hostId}
|
||||
route={{ pathname: `/h/${encodeURIComponent(hostId)}` }}
|
||||
route={route}
|
||||
fallback={<HostScreen />}
|
||||
/>
|
||||
)
|
||||
|
||||
@@ -1,6 +1,6 @@
|
||||
import { useLocalSearchParams } from 'expo-router'
|
||||
import { BridgeInitRouteSchema } from '../../../src/mobile-web-shell/bridge/bridge-envelope'
|
||||
import { MobileWebShellScreen } from '../../../src/mobile-web-shell/MobileWebShellScreen'
|
||||
import { shellScreenRoute } from '../../../src/mobile-web-shell/shell-screen-route'
|
||||
import { useMobileWebShellEnabled } from '../../../src/mobile-web-shell/use-mobile-web-shell-enabled'
|
||||
import { firstParam } from '../../../src/source-control/mobile-source-control-screen-state'
|
||||
import { MobileTasksScreen } from '../../../src/tasks/MobileTasksScreen'
|
||||
@@ -27,13 +27,13 @@ export default function MobileTasksRoute() {
|
||||
if (enabled !== true || !hostId) {
|
||||
return native
|
||||
}
|
||||
const route = {
|
||||
const route = shellScreenRoute({
|
||||
pathname: `/h/${encodeURIComponent(hostId)}/tasks`,
|
||||
// Omitted rather than empty: an absent provider lets the page pick its own default, where
|
||||
// `taskSource=` is a provider named nothing.
|
||||
...(taskSource === '' ? {} : { params: { taskSource } })
|
||||
}
|
||||
if (!BridgeInitRouteSchema.safeParse(route).success) {
|
||||
})
|
||||
if (route === null) {
|
||||
return native
|
||||
}
|
||||
return (
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { useMemo, useState } from 'react'
|
||||
import { Linking, Pressable, ScrollView, StyleSheet, Text, View } from 'react-native'
|
||||
import { Pressable, ScrollView, StyleSheet, Text, View } from 'react-native'
|
||||
import { openExternalLink } from '../../platform/external-link'
|
||||
import { ChevronDown, ChevronRight } from 'lucide-react-native'
|
||||
import { colors, radii, spacing, typography } from '../../theme/mobile-theme'
|
||||
import { MermaidDiagram } from './MermaidDiagram'
|
||||
@@ -145,7 +146,7 @@ function openMarkdownLink(url: string): void {
|
||||
if (!isAllowedMarkdownLinkUrl(url)) {
|
||||
return
|
||||
}
|
||||
void Linking.openURL(url).catch(() => {})
|
||||
openExternalLink(url)
|
||||
}
|
||||
|
||||
function alignToFlex(align: CellAlign | undefined): 'flex-start' | 'center' | 'flex-end' {
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { useCallback, useEffect, useRef, useState } from 'react'
|
||||
import { ActivityIndicator, Linking, Pressable, Text, View } from 'react-native'
|
||||
import { ActivityIndicator, Pressable, Text, View } from 'react-native'
|
||||
import { openExternalLink } from '../../platform/external-link'
|
||||
import { ChevronDown, ChevronRight, ExternalLink, RotateCw, Sparkles } from 'lucide-react-native'
|
||||
import { colors } from '../../theme/mobile-theme'
|
||||
import type { PRCheckDetail } from '../../../../src/shared/github/check-types'
|
||||
@@ -233,7 +234,7 @@ export function PRChecksSection({
|
||||
{url ? (
|
||||
<Pressable
|
||||
style={styles.rowTrailing}
|
||||
onPress={() => void Linking.openURL(url).catch(() => {})}
|
||||
onPress={() => openExternalLink(url)}
|
||||
hitSlop={6}
|
||||
accessibilityRole="button"
|
||||
accessibilityLabel={`Open ${check.name} on the web`}
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import { memo, useState } from 'react'
|
||||
import { Image, Linking, Pressable, Text, View } from 'react-native'
|
||||
import { Image, Pressable, Text, View } from 'react-native'
|
||||
import { openExternalLink } from '../../platform/external-link'
|
||||
import { Check, CornerDownRight, ExternalLink, Pencil, Trash2, Undo2 } from 'lucide-react-native'
|
||||
import type {
|
||||
GitHubReaction,
|
||||
@@ -140,7 +141,7 @@ export const PRCommentCard = memo(function PRCommentCard({
|
||||
{comment.url ? (
|
||||
<Pressable
|
||||
style={styles.openButton}
|
||||
onPress={() => void Linking.openURL(comment.url).catch(() => {})}
|
||||
onPress={() => openExternalLink(comment.url)}
|
||||
hitSlop={8}
|
||||
accessibilityRole="button"
|
||||
accessibilityLabel="Open comment on GitHub"
|
||||
|
||||
@@ -0,0 +1,106 @@
|
||||
import { createElement } from 'react'
|
||||
import { act, create, type ReactTestRenderer } from 'react-test-renderer'
|
||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
import type { PRInfo } from '../../../../src/shared/github/pull-request-types'
|
||||
import { PRConflictingFilesSection } from './PRConflictingFilesSection'
|
||||
import { buildMergeabilityRefreshCommands } from './pr-conflict-presentation'
|
||||
|
||||
const clipboard = vi.hoisted(() => ({ writeText: vi.fn() }))
|
||||
|
||||
vi.mock('react-native', () => ({
|
||||
ActivityIndicator: 'ActivityIndicator',
|
||||
Pressable: 'Pressable',
|
||||
ScrollView: 'ScrollView',
|
||||
StyleSheet: { create: (styles: unknown) => styles },
|
||||
Text: 'Text',
|
||||
View: 'View'
|
||||
}))
|
||||
|
||||
vi.mock('lucide-react-native', () => ({
|
||||
Check: 'Check',
|
||||
Copy: 'Copy',
|
||||
FileWarning: 'FileWarning',
|
||||
Sparkles: 'Sparkles'
|
||||
}))
|
||||
|
||||
vi.mock('../../platform/clipboard', () => ({ useClipboardWriter: () => clipboard }))
|
||||
|
||||
vi.mock('./PRSection', () => ({
|
||||
PRSection: ({ children }: { children: unknown }) => children
|
||||
}))
|
||||
|
||||
/**
|
||||
* A PR the resolver gives the refresh commands to, which is the only path that copies: the host
|
||||
* reports CONFLICTING, has no file list to show, and the local merge came back clean — the
|
||||
* disagreement the commands exist to resolve.
|
||||
*/
|
||||
const PR: Pick<PRInfo, 'mergeable' | 'conflictSummary'> = {
|
||||
mergeable: 'CONFLICTING',
|
||||
conflictSummary: {
|
||||
files: [],
|
||||
localMergeState: 'clean',
|
||||
commitsBehind: 2,
|
||||
baseCommit: 'abc1234',
|
||||
baseRef: 'main'
|
||||
}
|
||||
}
|
||||
|
||||
const COMMANDS = buildMergeabilityRefreshCommands()
|
||||
|
||||
function press(tree: ReactTestRenderer): Promise<void> {
|
||||
const control = tree.root
|
||||
.findAll((node) => node.props.accessibilityLabel === 'Copy mergeability refresh commands')
|
||||
.at(0)
|
||||
if (!control) {
|
||||
throw new Error('the copy control is not rendered')
|
||||
}
|
||||
return act(async () => control.props.onPress())
|
||||
}
|
||||
|
||||
function labels(tree: ReactTestRenderer): string[] {
|
||||
return tree.root
|
||||
.findAll((node) => typeof node.props.children === 'string')
|
||||
.flatMap((node) => (typeof node.props.children === 'string' ? [node.props.children] : []))
|
||||
}
|
||||
|
||||
describe('copying the mergeability refresh commands', () => {
|
||||
let tree: ReactTestRenderer | null = null
|
||||
|
||||
beforeEach(() => {
|
||||
clipboard.writeText.mockReset().mockResolvedValue(undefined)
|
||||
})
|
||||
|
||||
afterEach(() => {
|
||||
act(() => tree?.unmount())
|
||||
tree = null
|
||||
})
|
||||
|
||||
async function render(): Promise<ReactTestRenderer> {
|
||||
let rendered: ReactTestRenderer | null = null
|
||||
await act(async () => {
|
||||
rendered = create(createElement(PRConflictingFilesSection, { pr: PR }))
|
||||
})
|
||||
if (rendered === null) {
|
||||
throw new Error('the section did not render')
|
||||
}
|
||||
tree = rendered
|
||||
return rendered
|
||||
}
|
||||
|
||||
it('says it copied when the pasteboard took the commands', async () => {
|
||||
const rendered = await render()
|
||||
await press(rendered)
|
||||
expect(clipboard.writeText).toHaveBeenCalledWith(COMMANDS)
|
||||
expect(labels(rendered)).toContain('Copied')
|
||||
})
|
||||
|
||||
it('says it failed instead of saying nothing at all', async () => {
|
||||
// The seam rejects when the pasteboard refused, which inside the page is a route that was not
|
||||
// granted the verb. Dropped, the tap is indistinguishable from one that copied nothing.
|
||||
clipboard.writeText.mockRejectedValue(new Error('the clipboard did not accept this text'))
|
||||
const rendered = await render()
|
||||
await press(rendered)
|
||||
expect(labels(rendered)).toContain('Failed to copy text')
|
||||
expect(labels(rendered)).not.toContain('Copied')
|
||||
})
|
||||
})
|
||||
@@ -1,7 +1,7 @@
|
||||
import { useEffect, useRef, useState } from 'react'
|
||||
import { ActivityIndicator, Pressable, ScrollView, Text, View } from 'react-native'
|
||||
import * as Clipboard from 'expo-clipboard'
|
||||
import { Check, Copy, FileWarning, Sparkles } from 'lucide-react-native'
|
||||
import { useClipboardWriter } from '../../platform/clipboard'
|
||||
import { colors } from '../../theme/mobile-theme'
|
||||
import type { PRInfo } from '../../../../src/shared/github/pull-request-types'
|
||||
import { PRSection } from './PRSection'
|
||||
@@ -17,7 +17,9 @@ export type PrConflictsTriage = {
|
||||
}
|
||||
|
||||
type Props = {
|
||||
pr: PRInfo
|
||||
// What it reads, not the whole PR: the conflict view-model is the only thing derived here, and
|
||||
// a caller holding a full `PRInfo` satisfies this.
|
||||
pr: Pick<PRInfo, 'mergeable' | 'conflictSummary'>
|
||||
// True while a refresh is in flight, so the fallback notice can explain that
|
||||
// missing conflict file details may still be loading (desktop parity).
|
||||
isRefreshing?: boolean
|
||||
@@ -29,7 +31,12 @@ type Props = {
|
||||
// list is not yet available. Ports the desktop ConflictingFilesSection +
|
||||
// MergeConflictNotice into the mobile card shell.
|
||||
export function PRConflictingFilesSection({ pr, isRefreshing = false, triage }: Props) {
|
||||
const [commandsCopied, setCommandsCopied] = useState(false)
|
||||
// The seam, not `expo-clipboard`: inside the shell the page's own clipboard needs a secure
|
||||
// context, which the iOS custom scheme is not and Android's https is.
|
||||
const clipboard = useClipboardWriter()
|
||||
// Three states, not a boolean: a refused write used to be caught and dropped, so the tap was
|
||||
// indistinguishable from one that copied. The tasks page reports its refusals the same way.
|
||||
const [copyState, setCopyState] = useState<'idle' | 'copied' | 'failed'>('idle')
|
||||
const copiedResetTimerRef = useRef<ReturnType<typeof setTimeout> | null>(null)
|
||||
const conflict = resolveConflictDisplay(pr)
|
||||
|
||||
@@ -56,21 +63,29 @@ export function PRConflictingFilesSection({ pr, isRefreshing = false, triage }:
|
||||
if (!conflict.mergeabilityRefreshCommands) {
|
||||
return
|
||||
}
|
||||
let next: 'copied' | 'failed' = 'copied'
|
||||
try {
|
||||
await Clipboard.setStringAsync(conflict.mergeabilityRefreshCommands)
|
||||
await clipboard.writeText(conflict.mergeabilityRefreshCommands)
|
||||
} catch {
|
||||
return
|
||||
next = 'failed'
|
||||
}
|
||||
if (copiedResetTimerRef.current) {
|
||||
clearTimeout(copiedResetTimerRef.current)
|
||||
}
|
||||
setCommandsCopied(true)
|
||||
setCopyState(next)
|
||||
copiedResetTimerRef.current = setTimeout(() => {
|
||||
copiedResetTimerRef.current = null
|
||||
setCommandsCopied(false)
|
||||
setCopyState('idle')
|
||||
}, 1500)
|
||||
}
|
||||
|
||||
const copyLabel =
|
||||
copyState === 'copied'
|
||||
? 'Copied'
|
||||
: copyState === 'failed'
|
||||
? 'Failed to copy text'
|
||||
: 'Copy commands'
|
||||
|
||||
return (
|
||||
<PRSection title="Conflicts">
|
||||
{conflict.commitsBehind !== null && conflict.baseCommit !== null ? (
|
||||
@@ -97,14 +112,12 @@ export function PRConflictingFilesSection({ pr, isRefreshing = false, triage }:
|
||||
accessibilityRole="button"
|
||||
accessibilityLabel="Copy mergeability refresh commands"
|
||||
>
|
||||
{commandsCopied ? (
|
||||
{copyState === 'copied' ? (
|
||||
<Check size={13} color={colors.textPrimary} strokeWidth={2.2} />
|
||||
) : (
|
||||
<Copy size={13} color={colors.textPrimary} strokeWidth={2.2} />
|
||||
)}
|
||||
<Text style={styles.copyCommandText}>
|
||||
{commandsCopied ? 'Copied' : 'Copy commands'}
|
||||
</Text>
|
||||
<Text style={styles.copyCommandText}>{copyLabel}</Text>
|
||||
</Pressable>
|
||||
</View>
|
||||
<Text selectable style={styles.commandText}>
|
||||
|
||||
@@ -1,77 +1,34 @@
|
||||
import { readdirSync, readFileSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
import ts from 'typescript-api'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
callsRouteHandoff,
|
||||
importsExpoRouterValue,
|
||||
parse,
|
||||
productFiles
|
||||
} from '../navigation/router-seam-census.test-support'
|
||||
|
||||
const FILES_ROOT = import.meta.dirname
|
||||
|
||||
/**
|
||||
* Which modules here hold a router, so the census cannot pass by seeing nothing.
|
||||
*
|
||||
* Inside the shell's page a screen is one document standing in for one screen, and `useRouteHandoff`
|
||||
* is the only thing that knows which targets the page keeps and which it hands back to the app. A
|
||||
* screen holding expo-router's own `useRouter` posts no `navigate`, so a target outside the page
|
||||
* paints Unmatched over it and a target inside it still works — which is why this is a census and
|
||||
* not a behaviour test: the failure is invisible from either screen's own tests.
|
||||
* The walk and the two rules are the seam's, shared with every other domain that runs them; what
|
||||
* stays here is this domain's own evidence: which of its modules are meant to hold a router.
|
||||
*/
|
||||
const ROUTER_HOLDERS = ['MobileFilePreviewScreen.tsx', 'MobileFileExplorerPanel.tsx']
|
||||
|
||||
function productFiles(): string[] {
|
||||
return readdirSync(FILES_ROOT, { recursive: true, encoding: 'utf8' })
|
||||
.map((entry) => entry.replaceAll('\\', '/'))
|
||||
.filter((entry) => /\.tsx?$/.test(entry) && !/\.test\.tsx?$/.test(entry))
|
||||
}
|
||||
|
||||
function parse(name: string): ts.SourceFile {
|
||||
return ts.createSourceFile(
|
||||
name,
|
||||
readFileSync(join(FILES_ROOT, name), 'utf8'),
|
||||
ts.ScriptTarget.Latest,
|
||||
true,
|
||||
name.endsWith('.tsx') ? ts.ScriptKind.TSX : ts.ScriptKind.TS
|
||||
)
|
||||
}
|
||||
|
||||
/** Value imports only: a `import type { Href } from 'expo-router'` names no runtime router. */
|
||||
function importsExpoRouterValue(source: ts.SourceFile): boolean {
|
||||
return source.statements.some((statement) => {
|
||||
if (!ts.isImportDeclaration(statement) || statement.importClause?.isTypeOnly === true) {
|
||||
return false
|
||||
}
|
||||
const specifier = statement.moduleSpecifier
|
||||
return ts.isStringLiteral(specifier) && specifier.text === 'expo-router'
|
||||
})
|
||||
}
|
||||
|
||||
function callsRouteHandoff(source: ts.SourceFile): boolean {
|
||||
let found = false
|
||||
const visit = (node: ts.Node): void => {
|
||||
if (
|
||||
ts.isCallExpression(node) &&
|
||||
ts.isIdentifier(node.expression) &&
|
||||
node.expression.text === 'useRouteHandoff'
|
||||
) {
|
||||
found = true
|
||||
}
|
||||
ts.forEachChild(node, visit)
|
||||
}
|
||||
ts.forEachChild(source, visit)
|
||||
return found
|
||||
}
|
||||
|
||||
describe('the files domain reaches the router through the handoff seam', () => {
|
||||
const files = productFiles()
|
||||
const files = productFiles(FILES_ROOT)
|
||||
|
||||
it('walks the modules it is written against', () => {
|
||||
expect(files).toEqual(expect.arrayContaining(ROUTER_HOLDERS))
|
||||
})
|
||||
|
||||
it('imports no router from expo-router, which the page cannot hand a route back through', () => {
|
||||
expect(files.filter((name) => importsExpoRouterValue(parse(name)))).toEqual([])
|
||||
expect(files.filter((name) => importsExpoRouterValue(parse(FILES_ROOT, name)))).toEqual([])
|
||||
})
|
||||
|
||||
it('takes the router from useRouteHandoff at every screen that holds one', () => {
|
||||
expect(files.filter((name) => callsRouteHandoff(parse(name))).sort()).toEqual(
|
||||
expect(files.filter((name) => callsRouteHandoff(parse(FILES_ROOT, name))).sort()).toEqual(
|
||||
[...ROUTER_HOLDERS].sort()
|
||||
)
|
||||
})
|
||||
|
||||
@@ -6,7 +6,7 @@ import {
|
||||
import { shellRouteHref } from '../mobile-web-shell/bridge/page-bootstrap'
|
||||
import { stringifyRouteHref } from '../navigation/route-href'
|
||||
import { createMobileFilePreviewHref } from './mobile-file-preview-route'
|
||||
import { mobileFileShellRoute } from './mobile-file-shell-route'
|
||||
import { shellScreenRoute } from '../mobile-web-shell/shell-screen-route'
|
||||
|
||||
/**
|
||||
* Every shape of a real file path that the bridge's route vocabulary would refuse as a segment.
|
||||
@@ -59,7 +59,7 @@ function relativePathFromHref(href: string): string | null {
|
||||
|
||||
describe.each(HAZARD_PATHS)('a file path the route carries: %s', (relativePath) => {
|
||||
it('is a route the page can be given, and a pathname with no path in it', () => {
|
||||
const route = mobileFileShellRoute({
|
||||
const route = shellScreenRoute({
|
||||
pathname: '/h/host-1/files/preview/wt-1',
|
||||
params: { relativePath, source: 'worktree' }
|
||||
})
|
||||
|
||||
@@ -5,13 +5,15 @@ import { beforeEach, describe, expect, it, vi } from 'vitest'
|
||||
type RouteDependencies = {
|
||||
storage: Map<string, string>
|
||||
pathnames: string[]
|
||||
hostId: string
|
||||
hostId: string | string[]
|
||||
nativeRenders: number
|
||||
}
|
||||
|
||||
const dependencies = vi.hoisted((): RouteDependencies => ({
|
||||
storage: new Map(),
|
||||
pathnames: [],
|
||||
hostId: 'host-1'
|
||||
hostId: 'host-1',
|
||||
nativeRenders: 0
|
||||
}))
|
||||
|
||||
vi.mock('@react-native-async-storage/async-storage', () => ({
|
||||
@@ -31,7 +33,27 @@ vi.mock('../components/WorkspaceDetailPlaceholder', () => ({
|
||||
WorkspaceDetailPlaceholder: () => null
|
||||
}))
|
||||
|
||||
vi.mock('../host-screen/HostScreen', () => ({ HostScreen: () => null }))
|
||||
vi.mock('../host-screen/HostScreen', () => ({
|
||||
HostScreen: () => {
|
||||
dependencies.nativeRenders += 1
|
||||
return null
|
||||
}
|
||||
}))
|
||||
|
||||
// `firstParam` lives beside the source-control screen state, which imports the lucide barrel, and
|
||||
// that barrel's `LucideProvider` re-export is the gap the web build patches with a plugin. Nine
|
||||
// icons, named as the other suites name theirs; none of them renders here.
|
||||
vi.mock('lucide-react-native', () => ({
|
||||
ArrowDown: vi.fn(),
|
||||
ArrowDownUp: vi.fn(),
|
||||
ArrowUp: vi.fn(),
|
||||
Check: vi.fn(),
|
||||
CloudUpload: vi.fn(),
|
||||
GitBranch: vi.fn(),
|
||||
GitPullRequestArrow: vi.fn(),
|
||||
History: vi.fn(),
|
||||
RefreshCw: vi.fn()
|
||||
}))
|
||||
|
||||
vi.mock('../layout/responsive-layout', () => ({
|
||||
useResponsiveLayout: () => ({ isWideLayout: false })
|
||||
@@ -57,6 +79,7 @@ describe('the native worktree-list route that hands off to the shell', () => {
|
||||
beforeEach(() => {
|
||||
dependencies.storage.clear()
|
||||
dependencies.pathnames.length = 0
|
||||
dependencies.nativeRenders = 0
|
||||
dependencies.hostId = 'host-1'
|
||||
Object.assign(globalThis, { __DEV__: true })
|
||||
dependencies.storage.set('orca:mobileWebShellEnabled', 'true')
|
||||
@@ -73,4 +96,36 @@ describe('the native worktree-list route that hands off to the shell', () => {
|
||||
expect(decodeURIComponent((pathname ?? '').slice('/h/'.length)), hostId).toBe(hostId)
|
||||
}
|
||||
})
|
||||
|
||||
it('keeps a dot-segment host id native instead of handing over a route the page refuses', async () => {
|
||||
// `encodeURIComponent` leaves a dot alone and `%2e%2e` is a dot segment to the URL parser too,
|
||||
// so this one cannot be encoded into a pathname the bridge accepts. Handed over it reaches the
|
||||
// phone as an `init` naming no screen and the page paints "Update Orca to open this
|
||||
// workspace" over the native list that is sitting right behind this switch.
|
||||
for (const hostId of ['..', '.']) {
|
||||
dependencies.hostId = hostId
|
||||
dependencies.pathnames.length = 0
|
||||
dependencies.nativeRenders = 0
|
||||
await renderRoute()
|
||||
expect(BRIDGE_ROUTE_PATHNAME_PATTERN.test(`/h/${hostId}`), hostId).toBe(false)
|
||||
expect(dependencies.pathnames, hostId).toEqual([])
|
||||
expect(dependencies.nativeRenders, hostId).toBeGreaterThan(0)
|
||||
}
|
||||
})
|
||||
|
||||
it('opens the first of a repeated host id, never the pair joined into one', async () => {
|
||||
// Expo Router answers a repeated key with an array. Interpolated, `String(['a','b'])` is
|
||||
// `a,b` and `encodeURIComponent` makes that the single segment `a%2Cb`, which the bridge's
|
||||
// segment rule accepts — so the shell would open a page for a host nobody has.
|
||||
dependencies.hostId = ['host-1', 'host-2']
|
||||
await renderRoute()
|
||||
expect(dependencies.pathnames).toEqual(['/h/host-1'])
|
||||
})
|
||||
|
||||
it('stays native for an empty repeated host id, which names no host at all', async () => {
|
||||
dependencies.hostId = []
|
||||
await renderRoute()
|
||||
expect(dependencies.pathnames).toEqual([])
|
||||
expect(dependencies.nativeRenders).toBeGreaterThan(0)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -0,0 +1,169 @@
|
||||
import { readdirSync, readFileSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
import ts from 'typescript-api'
|
||||
import { describe, expect, it } from 'vitest'
|
||||
|
||||
const HOST_ROUTES = join(import.meta.dirname, '..', '..', 'app', 'h', '[hostId]')
|
||||
const PAGE_ROUTE_REGISTRY = join(
|
||||
import.meta.dirname,
|
||||
'..',
|
||||
'..',
|
||||
'..',
|
||||
'config',
|
||||
'scripts',
|
||||
'mobile-web-page-routes.mjs'
|
||||
)
|
||||
|
||||
/**
|
||||
* Every switch that hands a route to the shell asks whether the route is one the page can be
|
||||
* given, and asks it in one place.
|
||||
*
|
||||
* A route the schema refuses is dropped to `null` by `bridge-host.ts` and reaches the phone as an
|
||||
* `init` naming no screen, which the page answers with "Update Orca to open this workspace" — a
|
||||
* failure screen in place of the native screen sitting right behind the switch. Three routes had
|
||||
* each grown their own copy of the call and two had none at all, which is the state this census
|
||||
* ends: the predicate is `shellScreenRoute`, and a switch that spells it itself has a second
|
||||
* spelling of a rule that can only drift from the one the page reads.
|
||||
*
|
||||
* The walk is over the route tree rather than a list, so a route added later is held to this
|
||||
* without anyone remembering to add it here.
|
||||
*/
|
||||
function hostRouteFiles(directory: string = HOST_ROUTES, prefix = ''): string[] {
|
||||
return readdirSync(directory, { withFileTypes: true }).flatMap((entry) => {
|
||||
const name = prefix === '' ? entry.name : `${prefix}/${entry.name}`
|
||||
if (entry.isDirectory()) {
|
||||
return hostRouteFiles(join(directory, entry.name), name)
|
||||
}
|
||||
return entry.name.endsWith('.tsx') && !entry.name.includes('.test.') ? [name] : []
|
||||
})
|
||||
}
|
||||
|
||||
const read = (name: string): string => readFileSync(join(HOST_ROUTES, name), 'utf8')
|
||||
|
||||
/**
|
||||
* The one switch that hands over a route the rule refuses, on purpose.
|
||||
*
|
||||
* `web.tsx` is `__DEV__`-only and its fallback is `Redirect href="/h/<hostId>"`, not a native
|
||||
* screen. Adopting the guard there sends a `..` deep link through that redirect to the host route,
|
||||
* which this PR keeps native, so the developer lands on the host list with nothing said about why
|
||||
* the page did not open. Handed over instead, the same id reaches the bridge and comes back as the
|
||||
* host's own failure screen, which is the better verdict for a route whose whole purpose is to
|
||||
* open the page deliberately; `mobile-web-shell-route.test.tsx` pins that by name.
|
||||
*
|
||||
* Exempted here rather than silently unwalked, so the exception is read when it changes.
|
||||
*/
|
||||
const HANDS_OVER_UNJUDGED = ['web.tsx']
|
||||
|
||||
const parse = (source: string): ts.SourceFile =>
|
||||
ts.createSourceFile('route.tsx', source, ts.ScriptTarget.Latest, true, ts.ScriptKind.TSX)
|
||||
|
||||
function importsNames(parsed: ts.SourceFile, module: string): string[] {
|
||||
return parsed.statements.flatMap((statement) => {
|
||||
if (!ts.isImportDeclaration(statement) || !ts.isStringLiteral(statement.moduleSpecifier)) {
|
||||
return []
|
||||
}
|
||||
if (!statement.moduleSpecifier.text.endsWith(module)) {
|
||||
return []
|
||||
}
|
||||
const bindings = statement.importClause?.namedBindings
|
||||
return bindings !== undefined && ts.isNamedImports(bindings)
|
||||
? bindings.elements.map((element) => element.name.text)
|
||||
: []
|
||||
})
|
||||
}
|
||||
|
||||
/**
|
||||
* A switch is a route file that imports the shell screen.
|
||||
*
|
||||
* The import rather than `<MobileWebShellScreen` in the text: a switch that renders it through an
|
||||
* alias, or across a line break the formatter chose, is still a switch, and a file that only
|
||||
* mentions the name in a comment is not one.
|
||||
*/
|
||||
function mountsShell(parsed: ts.SourceFile): boolean {
|
||||
return importsNames(parsed, 'MobileWebShellScreen').includes('MobileWebShellScreen')
|
||||
}
|
||||
|
||||
/**
|
||||
* Whether the module calls the name it imported, not merely imports it.
|
||||
*
|
||||
* An import alone is what a switch has while the call sits behind a condition that never runs, or
|
||||
* after someone deletes the call and leaves the import for the formatter to trim later. The rule
|
||||
* is that every switch asks, so the census reads the asking.
|
||||
*/
|
||||
function callsName(parsed: ts.SourceFile, name: string): boolean {
|
||||
let called = false
|
||||
const visit = (node: ts.Node): void => {
|
||||
if (
|
||||
ts.isCallExpression(node) &&
|
||||
ts.isIdentifier(node.expression) &&
|
||||
node.expression.text === name
|
||||
) {
|
||||
called = true
|
||||
}
|
||||
ts.forEachChild(node, visit)
|
||||
}
|
||||
ts.forEachChild(parsed, visit)
|
||||
return called
|
||||
}
|
||||
|
||||
describe('the switches that hand a route to the shell', () => {
|
||||
const switches = hostRouteFiles().filter((name) => mountsShell(parse(read(name))))
|
||||
|
||||
it('walks the directory every registered route lives under', () => {
|
||||
// The root above is written out rather than derived, so this is what ties it to the manifest:
|
||||
// a page route outside `/h/[hostId]` would be a switch this census never reads.
|
||||
const pathnames = [
|
||||
...readFileSync(PAGE_ROUTE_REGISTRY, 'utf8').matchAll(/pathname: '([^']+)'/g)
|
||||
].map((match) => match[1])
|
||||
expect(pathnames.length).toBeGreaterThan(0)
|
||||
expect(pathnames.filter((pathname) => !pathname.startsWith('/h/[hostId]'))).toEqual([])
|
||||
})
|
||||
|
||||
it('walks the route tree and finds them, so the rules below cannot pass vacuously', () => {
|
||||
expect(switches.sort()).toEqual([
|
||||
'agent-history/[worktreeId].tsx',
|
||||
'files/[worktreeId].tsx',
|
||||
'files/preview/[worktreeId].tsx',
|
||||
'index.tsx',
|
||||
'tasks.tsx',
|
||||
'web.tsx'
|
||||
])
|
||||
})
|
||||
|
||||
it('asks shellScreenRoute whether the route is one the page can be given', () => {
|
||||
// Imports it *and* calls it: an import the call no longer reaches is a switch that stopped
|
||||
// asking while still looking like one.
|
||||
expect(
|
||||
switches
|
||||
.filter((name) => !HANDS_OVER_UNJUDGED.includes(name))
|
||||
.filter((name) => {
|
||||
const parsed = parse(read(name))
|
||||
return (
|
||||
!importsNames(parsed, 'shell-screen-route').includes('shellScreenRoute') ||
|
||||
!callsName(parsed, 'shellScreenRoute')
|
||||
)
|
||||
})
|
||||
).toEqual([])
|
||||
})
|
||||
|
||||
it('reads the call rather than the import, on a fixture that has only the import', () => {
|
||||
// The rule the case above cannot show against the tree, every switch there calling what it
|
||||
// imports: an import with no call is named.
|
||||
const importOnly = parse(
|
||||
"import { shellScreenRoute } from '../../../src/mobile-web-shell/shell-screen-route'\n" +
|
||||
"import { MobileWebShellScreen } from '../../../src/mobile-web-shell/MobileWebShellScreen'\n" +
|
||||
'export default function Route() {\n return <MobileWebShellScreen />\n}\n'
|
||||
)
|
||||
expect(mountsShell(importOnly)).toBe(true)
|
||||
expect(importsNames(importOnly, 'shell-screen-route')).toContain('shellScreenRoute')
|
||||
expect(callsName(importOnly, 'shellScreenRoute')).toBe(false)
|
||||
})
|
||||
|
||||
it('spells the rule nowhere else, so the page and the app cannot disagree about it', () => {
|
||||
// The copies this census ends. `shellScreenRoute` is the one caller of the schema outside the
|
||||
// bridge, and a switch that reaches for it again is writing the second spelling back.
|
||||
expect(switches.filter((name) => read(name).includes('BridgeInitRouteSchema'))).toEqual([])
|
||||
// And the exemption names a switch that exists, so it cannot outlive the file it excuses.
|
||||
expect(switches).toEqual(expect.arrayContaining(HANDS_OVER_UNJUDGED))
|
||||
})
|
||||
})
|
||||
+11
-14
@@ -1,15 +1,12 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import { BRIDGE_MAX_ROUTE_PARAM_CHARS } from '../mobile-web-shell/bridge/bridge-caps'
|
||||
import {
|
||||
BridgeInitRouteSchema,
|
||||
type BridgeInitRoute
|
||||
} from '../mobile-web-shell/bridge/bridge-envelope'
|
||||
import { shellRouteHref } from '../mobile-web-shell/bridge/page-bootstrap'
|
||||
import { mobileFileShellRoute, mobileFileShellRouteKey } from './mobile-file-shell-route'
|
||||
import { BRIDGE_MAX_ROUTE_PARAM_CHARS } from './bridge/bridge-caps'
|
||||
import { BridgeInitRouteSchema, type BridgeInitRoute } from './bridge/bridge-envelope'
|
||||
import { shellRouteHref } from './bridge/page-bootstrap'
|
||||
import { shellScreenRoute, shellScreenRouteKey } from './shell-screen-route'
|
||||
import {
|
||||
mobileFilePreviewShellParams,
|
||||
normalizeMobileFilePreviewRouteParams
|
||||
} from './mobile-file-preview-route'
|
||||
} from '../files/mobile-file-preview-route'
|
||||
|
||||
const PREVIEW_PATH = '/h/host-1/files/preview/wt-1'
|
||||
|
||||
@@ -27,11 +24,11 @@ function previewRoute(absolutePath: string) {
|
||||
return { pathname: PREVIEW_PATH, params: mobileFilePreviewShellParams(route.params) }
|
||||
}
|
||||
|
||||
describe('the route the files screens hand the shell', () => {
|
||||
describe('the route a switch hands the shell', () => {
|
||||
it('is one the page could actually be given', () => {
|
||||
const route = previewRoute('/logs/run.txt')
|
||||
expect(BridgeInitRouteSchema.safeParse(route).success).toBe(true)
|
||||
expect(mobileFileShellRoute(route)).toEqual(route)
|
||||
expect(shellScreenRoute(route)).toEqual(route)
|
||||
})
|
||||
|
||||
it('is nothing when a file path is longer than a param may be', () => {
|
||||
@@ -41,7 +38,7 @@ describe('the route the files screens hand the shell', () => {
|
||||
// workspace" over a native screen that works.
|
||||
const route = previewRoute(`/logs/${'a'.repeat(BRIDGE_MAX_ROUTE_PARAM_CHARS)}.txt`)
|
||||
expect(BridgeInitRouteSchema.safeParse(route).success).toBe(false)
|
||||
expect(mobileFileShellRoute(route)).toBeNull()
|
||||
expect(shellScreenRoute(route)).toBeNull()
|
||||
})
|
||||
|
||||
it('is nothing when a worktree id is not a segment the page will route', () => {
|
||||
@@ -51,7 +48,7 @@ describe('the route the files screens hand the shell', () => {
|
||||
// The schema first, as the length case does: without it a `null` here would also be what a
|
||||
// guard that refused everything produces.
|
||||
expect(BridgeInitRouteSchema.safeParse(route).success).toBe(false)
|
||||
expect(mobileFileShellRoute(route)).toBeNull()
|
||||
expect(shellScreenRoute(route)).toBeNull()
|
||||
})
|
||||
|
||||
it('keeps a path with a slash, a space and a dot segment, which are params and not segments', () => {
|
||||
@@ -59,7 +56,7 @@ describe('the route the files screens hand the shell', () => {
|
||||
pathname: '/h/host-1/files/preview/wt-1',
|
||||
params: { relativePath: 'docs/../my notes/readme.md', source: 'worktree' }
|
||||
}
|
||||
expect(mobileFileShellRoute(route)).toEqual(route)
|
||||
expect(shellScreenRoute(route)).toEqual(route)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -77,6 +74,6 @@ describe('the key a shell screen remounts on', () => {
|
||||
params: { relativePath: 'docs/my notes/readme.md', source: 'worktree', line: '12' }
|
||||
}
|
||||
])('is the href the page would write into its history: %o', (route) => {
|
||||
expect(mobileFileShellRouteKey(route)).toBe(shellRouteHref(route))
|
||||
expect(shellScreenRouteKey(route)).toBe(shellRouteHref(route))
|
||||
})
|
||||
})
|
||||
+10
-12
@@ -1,7 +1,4 @@
|
||||
import {
|
||||
BridgeInitRouteSchema,
|
||||
type BridgeInitRoute
|
||||
} from '../mobile-web-shell/bridge/bridge-envelope'
|
||||
import { BridgeInitRouteSchema, type BridgeInitRoute } from './bridge/bridge-envelope'
|
||||
|
||||
/**
|
||||
* The route to hand the shell, or nothing if the page could not be given it.
|
||||
@@ -12,16 +9,17 @@ import {
|
||||
* native screen sitting right behind the switch. Deciding here instead means the route stays
|
||||
* native, which is where every route starts.
|
||||
*
|
||||
* A file path is the reason this domain needs it. Paths are params, not segments, so `/`, spaces
|
||||
* and `..` are all fine; length is not bounded by anything the user cannot exceed, and
|
||||
* `BRIDGE_MAX_ROUTE_PARAM_CHARS` is 1024 while a Windows long path is not. The same call also
|
||||
* catches a `worktreeId` the segment rule refuses, which is the C1.8 class.
|
||||
* A file path is the reason the files routes needed it first. Paths are params, not segments, so
|
||||
* `/`, spaces and `..` are all fine; length is not bounded by anything the user cannot exceed,
|
||||
* and `BRIDGE_MAX_ROUTE_PARAM_CHARS` is 1024 while a Windows long path is not. The same call
|
||||
* also catches a `worktreeId` the segment rule refuses, which is the C1.8 class, and a host id
|
||||
* that encoding does not save — a `.` or `..` — which is why every switch asks it now.
|
||||
*
|
||||
* The schema itself is the predicate rather than a copy of its bounds: two spellings of one rule
|
||||
* drift, and the half that matters is the half the page reads. This belongs in the shell beside
|
||||
* that schema; it lives here while the contract files are the C2 lane's.
|
||||
* drift, and the half that matters is the half the page reads. Here rather than in one domain
|
||||
* because three routes had grown their own copy of the call.
|
||||
*/
|
||||
export function mobileFileShellRoute(route: BridgeInitRoute): BridgeInitRoute | null {
|
||||
export function shellScreenRoute(route: BridgeInitRoute): BridgeInitRoute | null {
|
||||
return BridgeInitRouteSchema.safeParse(route).success ? route : null
|
||||
}
|
||||
|
||||
@@ -41,7 +39,7 @@ export function mobileFileShellRoute(route: BridgeInitRoute): BridgeInitRoute |
|
||||
* document channel, and a native route file must not pull those into the app. The test pins the
|
||||
* two equal instead, which is the dependency this comment actually has.
|
||||
*/
|
||||
export function mobileFileShellRouteKey(route: BridgeInitRoute): string {
|
||||
export function shellScreenRouteKey(route: BridgeInitRoute): string {
|
||||
const search = new URLSearchParams(route.params ?? {}).toString()
|
||||
return search === '' ? route.pathname : `${route.pathname}?${search}`
|
||||
}
|
||||
@@ -0,0 +1,61 @@
|
||||
import { readdirSync, readFileSync } from 'node:fs'
|
||||
import { join } from 'node:path'
|
||||
import ts from 'typescript-api'
|
||||
|
||||
/**
|
||||
* How a domain census reads which of its modules hold a router, and where each one got it.
|
||||
*
|
||||
* Inside the shell's page a screen is one document standing in for one screen, and
|
||||
* `useRouteHandoff` is the only thing that knows which targets the page keeps and which it hands
|
||||
* back to the app. A screen holding expo-router's own `useRouter` posts no `navigate`, so a target
|
||||
* outside the page paints Unmatched over it and a target inside it still works — which is why this
|
||||
* is a census and not a behaviour test: the failure is invisible from either screen's own tests.
|
||||
*
|
||||
* Shared by every domain that runs it rather than copied per domain: C3.1 wrote this walk for the
|
||||
* files tree and the source-control tree wanted the same four rules, and two spellings of one rule
|
||||
* drift apart in exactly the half nobody reads again.
|
||||
*/
|
||||
|
||||
/** Every product module under a domain root, as paths relative to it. */
|
||||
export function productFiles(root: string): string[] {
|
||||
return readdirSync(root, { recursive: true, encoding: 'utf8' })
|
||||
.map((entry) => entry.replaceAll('\\', '/'))
|
||||
.filter((entry) => /\.tsx?$/.test(entry) && !/\.test\.tsx?$/.test(entry))
|
||||
}
|
||||
|
||||
export function parse(root: string, name: string): ts.SourceFile {
|
||||
return ts.createSourceFile(
|
||||
name,
|
||||
readFileSync(join(root, name), 'utf8'),
|
||||
ts.ScriptTarget.Latest,
|
||||
true,
|
||||
name.endsWith('.tsx') ? ts.ScriptKind.TSX : ts.ScriptKind.TS
|
||||
)
|
||||
}
|
||||
|
||||
/** Value imports only: an `import type { Href } from 'expo-router'` names no runtime router. */
|
||||
export function importsExpoRouterValue(source: ts.SourceFile): boolean {
|
||||
return source.statements.some((statement) => {
|
||||
if (!ts.isImportDeclaration(statement) || statement.importClause?.isTypeOnly === true) {
|
||||
return false
|
||||
}
|
||||
const specifier = statement.moduleSpecifier
|
||||
return ts.isStringLiteral(specifier) && specifier.text === 'expo-router'
|
||||
})
|
||||
}
|
||||
|
||||
export function callsRouteHandoff(source: ts.SourceFile): boolean {
|
||||
let found = false
|
||||
const visit = (node: ts.Node): void => {
|
||||
if (
|
||||
ts.isCallExpression(node) &&
|
||||
ts.isIdentifier(node.expression) &&
|
||||
node.expression.text === 'useRouteHandoff'
|
||||
) {
|
||||
found = true
|
||||
}
|
||||
ts.forEachChild(node, visit)
|
||||
}
|
||||
ts.forEachChild(source, visit)
|
||||
return found
|
||||
}
|
||||
@@ -14,7 +14,10 @@ import { useMobileDiffReviewSendActions } from './use-mobile-diff-review-send-ac
|
||||
type SendActions = ReturnType<typeof useMobileDiffReviewSendActions>
|
||||
|
||||
vi.mock('../platform/haptics', () => ({ triggerSuccess: vi.fn() }))
|
||||
vi.mock('expo-clipboard', () => ({ setStringAsync: vi.fn().mockResolvedValue(undefined) }))
|
||||
// Resolving `true`, which is what the pasteboard answers when it took the text: the seam reads
|
||||
// that boolean, and a mock resolving `undefined` put every copy down the refusal arm unseen.
|
||||
const clipboardMock = vi.hoisted(() => ({ setStringAsync: vi.fn() }))
|
||||
vi.mock('expo-clipboard', () => clipboardMock)
|
||||
|
||||
function sendResponse(accepted: boolean) {
|
||||
return {
|
||||
@@ -52,6 +55,7 @@ describe('useMobileDiffReviewSendActions', () => {
|
||||
let saveCommentsAndReviewState: ReturnType<typeof vi.fn>
|
||||
|
||||
beforeEach(() => {
|
||||
clipboardMock.setStringAsync.mockReset().mockResolvedValue(true)
|
||||
resetMobileNativeChatStaleInputForTests()
|
||||
setActionError = vi.fn()
|
||||
setSendSheet = vi.fn()
|
||||
@@ -85,6 +89,34 @@ describe('useMobileDiffReviewSendActions', () => {
|
||||
})
|
||||
}
|
||||
|
||||
/** Copying reaches no client, so the cases below mount without one rather than stubbing it. */
|
||||
async function mountWithoutClient(): Promise<void> {
|
||||
mountedClient = null
|
||||
await act(async () => {
|
||||
renderer = create(createElement(Harness))
|
||||
})
|
||||
}
|
||||
|
||||
it('copies the notes through the platform seam and says so', async () => {
|
||||
await mountWithoutClient()
|
||||
await act(async () => {
|
||||
await actions?.copyNotes()
|
||||
})
|
||||
expect(clipboardMock.setStringAsync).toHaveBeenCalledOnce()
|
||||
expect(setActionError).toHaveBeenLastCalledWith('Review notes copied')
|
||||
})
|
||||
|
||||
it('reports a refused copy instead of claiming it copied', async () => {
|
||||
// The pasteboard answering `false` is the case the seam exists to surface: on the web the verb
|
||||
// is refused when the route was not granted it, and the only caller is a floating promise.
|
||||
clipboardMock.setStringAsync.mockResolvedValue(false)
|
||||
await mountWithoutClient()
|
||||
await act(async () => {
|
||||
await actions?.copyNotes()
|
||||
})
|
||||
expect(setActionError).toHaveBeenLastCalledWith('the clipboard did not accept this text')
|
||||
})
|
||||
|
||||
it('heals a marked terminal BEFORE submitting the notes', async () => {
|
||||
const sendRequest = vi.fn().mockResolvedValue(sendResponse(true))
|
||||
await mount({ sendRequest } as unknown as RpcClient)
|
||||
|
||||
@@ -1,8 +1,8 @@
|
||||
import { useCallback, type Dispatch, type SetStateAction } from 'react'
|
||||
import * as Clipboard from 'expo-clipboard'
|
||||
import type { DiffComment, MobileDiffReviewState } from '../../../src/shared/diff-comment-types'
|
||||
import type { ConnectionState } from '../transport/types'
|
||||
import type { RpcClient } from '../transport/rpc-client'
|
||||
import { useClipboardWriter } from '../platform/clipboard'
|
||||
import { triggerSuccess } from '../platform/haptics'
|
||||
import { formatDiffComments, formatMobileDiffReviewPrompt } from './mobile-diff-comments'
|
||||
import { clearSentMobileDiffComments, markMobileDiffCommentsSent } from './mobile-diff-comment-edit'
|
||||
@@ -29,6 +29,9 @@ type SendActionsInput = {
|
||||
}
|
||||
|
||||
export function useMobileDiffReviewSendActions(input: SendActionsInput) {
|
||||
// The seam, not `expo-clipboard`: inside the shell the page's own clipboard needs a secure
|
||||
// context, which the iOS custom scheme is not and Android's https is.
|
||||
const clipboard = useClipboardWriter()
|
||||
const {
|
||||
client,
|
||||
connState,
|
||||
@@ -43,10 +46,17 @@ export function useMobileDiffReviewSendActions(input: SendActionsInput) {
|
||||
if (screenState.kind !== 'ready' || screenState.comments.length === 0) {
|
||||
return
|
||||
}
|
||||
await Clipboard.setStringAsync(formatDiffComments(screenState.comments))
|
||||
// Caught here because the only caller is `void controller.copyNotes()`: the seam rejects when
|
||||
// the pasteboard refused, and an uncaught rejection would leave "copied" as the last word.
|
||||
try {
|
||||
await clipboard.writeText(formatDiffComments(screenState.comments))
|
||||
} catch (err) {
|
||||
setActionError(err instanceof Error ? err.message : 'Unable to copy the review notes')
|
||||
return
|
||||
}
|
||||
triggerSuccess()
|
||||
setActionError('Review notes copied')
|
||||
}, [screenState, setActionError])
|
||||
}, [clipboard, screenState, setActionError])
|
||||
|
||||
const clearSentNotes = useCallback(async () => {
|
||||
if (screenState.kind !== 'ready') {
|
||||
|
||||
@@ -0,0 +1,44 @@
|
||||
import { describe, expect, it } from 'vitest'
|
||||
import {
|
||||
callsRouteHandoff,
|
||||
importsExpoRouterValue,
|
||||
parse,
|
||||
productFiles
|
||||
} from '../navigation/router-seam-census.test-support'
|
||||
|
||||
const SOURCE_CONTROL_ROOT = import.meta.dirname
|
||||
|
||||
/**
|
||||
* Which modules here hold a router, so the census cannot pass by seeing nothing.
|
||||
*
|
||||
* One holder, not one per screen: the hub's router is taken once in the openers hook and passed
|
||||
* down through the state hook to the runners and the panel. So this domain's whole reach into the
|
||||
* router is that single call, and the rules below say so rather than counting screens.
|
||||
*
|
||||
* `use-mobile-source-control-runners.ts` is the case a value/type rule is written for: it named
|
||||
* expo-router only to write `ReturnType<typeof useRouter>`, which is a value import in a type
|
||||
* position and keeps the module in the graph. `RouteHandoff` is the seam's own name for that type.
|
||||
*/
|
||||
const ROUTER_HOLDERS = ['use-mobile-source-control-openers.ts']
|
||||
|
||||
describe('the source-control domain reaches the router through the handoff seam', () => {
|
||||
const files = productFiles(SOURCE_CONTROL_ROOT)
|
||||
|
||||
it('walks the modules it is written against', () => {
|
||||
expect(files).toEqual(expect.arrayContaining(ROUTER_HOLDERS))
|
||||
expect(files).toContain('use-mobile-source-control-runners.ts')
|
||||
expect(files).toContain('MobileSourceControlPanel.tsx')
|
||||
})
|
||||
|
||||
it('imports no router from expo-router, which the page cannot hand a route back through', () => {
|
||||
expect(
|
||||
files.filter((name) => importsExpoRouterValue(parse(SOURCE_CONTROL_ROOT, name)))
|
||||
).toEqual([])
|
||||
})
|
||||
|
||||
it('takes the router from useRouteHandoff at every screen that holds one', () => {
|
||||
expect(
|
||||
files.filter((name) => callsRouteHandoff(parse(SOURCE_CONTROL_ROOT, name))).sort()
|
||||
).toEqual([...ROUTER_HOLDERS].sort())
|
||||
})
|
||||
})
|
||||
@@ -1,5 +1,5 @@
|
||||
import { useCallback, useRef, useState, type MutableRefObject } from 'react'
|
||||
import { useRouter } from 'expo-router'
|
||||
import { useRouteHandoff } from '../navigation/route-handoff'
|
||||
import type { RpcClient } from '../transport/rpc-client'
|
||||
import { refusedRpcMessageOrFallback } from '../transport/rpc-refusal-message'
|
||||
import type { ConnectionState } from '../transport/types'
|
||||
@@ -67,7 +67,9 @@ export function useMobileSourceControlOpeners(params: Params) {
|
||||
busyActionRef,
|
||||
setActionError
|
||||
} = params
|
||||
const router = useRouter()
|
||||
// The seam, not expo-router's own: inside the shell's page a push to a route the page does not
|
||||
// render has to be handed back to the app, and only this knows which targets those are.
|
||||
const router = useRouteHandoff()
|
||||
const [branchDiffPreview, setBranchDiffPreview] = useState<MobileBranchDiffPreviewState | null>(
|
||||
null
|
||||
)
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
import { useCallback, type MutableRefObject } from 'react'
|
||||
import { useRouter } from 'expo-router'
|
||||
import type { RouteHandoff } from '../navigation/route-handoff'
|
||||
import type { RpcClient } from '../transport/rpc-client'
|
||||
import { triggerError, triggerSuccess } from '../platform/haptics'
|
||||
import { useMobileCommitMessageGeneration } from './use-mobile-commit-message-generation'
|
||||
@@ -28,7 +28,7 @@ type Params = {
|
||||
generatingMessage: boolean
|
||||
stageablePaths: string[]
|
||||
unstageablePaths: string[]
|
||||
router: ReturnType<typeof useRouter>
|
||||
router: RouteHandoff
|
||||
sendGitRequest: SendGitRequest
|
||||
sendCommitRequest: (message: string) => Promise<unknown>
|
||||
runGitSyncSteps: () => Promise<void>
|
||||
|
||||
Reference in New Issue
Block a user