Document development builds; Expo Go no longer runs this app - #140
Document development builds; Expo Go no longer runs this app#140Jberma23 wants to merge 1 commit into
Conversation
|
Added a second commit: Running on a physical iPhone, plus a warning about the The README got someone as far as a simulator and stopped, which for this app is not far enough. The 0015 unlock path cannot be checked in a simulator at all — simulators do not enforce biometric authentication when retrieving a stored secret, which expo-secure-store documents — and nothing in the test suite shows the database is encrypted, since It also states plainly that a free Apple ID is enough — no push, no app groups, no associated domains, so nothing here needs a paid account. Worth writing down, because assuming otherwise is what quietly stops a volunteer from testing on the phone already in their pocket. Then signing, trusting the certificate, going wireless after the first cable build, the seven-day expiry, and One thing that belongs in #128, not here
That is a decision, not a prompt to click through at the end of a long setup. I have documented it defensively ("do not invent one, ask first"), but the real fix is setting it deliberately — something like Checked again after the edit: 18 code fences balanced, all five ADR links resolve, dash style normalised to match the rest of the file. |
|
npx expo start picks its launch target from that dependency too, so plain npm start targets Expo Go and pressing i/a opens Expo Go, not the build you installed. run:ios forces --dev-client itself, so the first build is fine; it's the next morning that misfires. Intentional, or worth changing the daily loop to npx expo start --dev-client? Same question for npm run ios / npm run android under Other command. |
Not intentional — good catch. start now passes --dev-client, ios/android are now run:ios/run:android, and the READMEs explain the flag. Worth knowing the misfire was silent: Expo Go loads this app fine and stock SQLite ignores PRAGMA key, so it'd write the journal unencrypted with no error. |
9edfed6 to
8350d03
Compare
Rebased onto main after #128 merged. Main's own follow-up commit (a8afb2a) had already rewritten much of this README for development builds, so rather than replay three commits written against the pre-merge tree, this keeps main's version as the base and folds in the parts it does not have. The one behaviour change is `npm start`, now `expo start --dev-client`. Without the flag, `expo start` picks its target by whether `expo-dev-client` is a dependency — it is not — so a bare `npm start` installs and opens Expo Go, which is the build that writes the journal in cleartext. `run:ios` and `run:android` pass the flag themselves, which is why the first day works and the morning after does not. The README said `npm start` "picks it up, the same as it always did"; that was the one line describing the daily loop, and it was wrong. The docs this adds on top of main: - Why Expo Go is worse than "unsupported" — it loads, renders, and writes plaintext, because stock SQLite ignores `PRAGMA key` rather than erroring. - Prerequisites split per platform, and Node 22.x recommended up front rather than only in the tests section. - `prebuild` asks for a bundle identifier the first time, and the answer is permanent — do not invent one to clear the prompt. - A "Running on a physical iPhone" section: free Apple ID provisioning is enough, how to fix signing, the seven-day expiry, and the three things (Face ID, real encryption, Keychain) no simulator or CI run can tell you. - That `npm run web` is the landing surface only, and that demoing to non-technical people no longer works through Expo Go. Closes #129
ffc4227 to
5e32f62
Compare
|
Rebased onto Heads up that this is not a plain replay of the three commits, because Kept from this PR (all still absent from
Dropped as already on One change is not docs. Two forward references from the original description are now resolved: #133 is fixed on Verified locally on Node 22.14: Still unverified, and still the acceptance criterion that matters most: that someone with a clean checkout can follow this start to finish and reach a running app. That needs macOS + Xcode and an Android Studio machine. #129's QA checklist covers it and someone should walk it before this merges. |
What this does
Rewrites
mobile/README.mdfor development builds. The README still described the old workflow — install Expo Go,npm start, scan a QR code — which withuseSQLCipherset is instructions for something that cannot work, aimed at exactly the newcomers 0008 designed a gentle first ticket for.The constraint now leads rather than being buried, because it decides whether someone can work on this at all: a new "This app does not run in Expo Go" section up front, saying that SQLCipher is compiled in at build time, that the first build is slow, and that Xcode or Android Studio is now a prerequisite rather than an optional extra. Setup is
npm install→npx expo prebuild→npx expo run:ios/run:android, with the day-to-day loop after that separated out so nobody thinks they rebuild every morning. Prerequisites say which toolchain each platform needs and that iOS has no path without a Mac.Two other things the encrypted database made untrue:
npm run webis documented as the landing surface only. Per 0017 the journal refuses to open in a browser rather than quietly writing an unencrypted medical journal into browser storage, so any screen reading the journal throws there by design. Someone who runs it and sees the landing page render would reasonably conclude the opposite.Smaller corrections while in there:
src/lib/db/andsrc/lib/auth/added to the project structure, which was silently missing the largest new directory; and the tests section now says that tests need no native build, that two suites want Node 22.5, and that a green suite does not prove the file is encrypted.Issue
Closes #129
Testing
No code changed, so nothing to unit test. What I did check:
../docs/decisions/*.mdlinks resolve against the files on this branch.expo-dev-clientis not a dependency, sonpx expo run:ios/run:androidis the correct path rather than anything requiring it, and/ios+/androidare gitignored, matching 0016's "generated rather than committed".The acceptance criterion I cannot verify is the one that matters most: that a contributor with a clean checkout can follow this start to finish and reach a running app. That needs macOS + Xcode and an Android Studio machine, neither of which I have. #129's QA checklist covers it and someone should walk it before this merges.
Notes for review
feature/encrypted-storage-unlock-foundation, notmain. 0016 asks for this to land with or close behind Add encrypted storage and biometric unlock foundation #128, and onmaintodayuseSQLCipheris not set — so merged to main first, this README would describe a requirement that does not exist yet. Targeting Paul's branch keeps Add encrypted storage and biometric unlock foundation #128 self-consistent: the PR that breaks Expo Go also fixes the docs describing Expo Go.node:sqlitesuites currently error rather than skip on Node below 22.5, and links jest node:sqlite guard never runs — two suites fail instead of skipping on Node < 22.5 #133. That is true on this branch; jest node:sqlite guard never runs — two suites fail instead of skipping on Node < 22.5 #133 is fixed in Address #128 review: unopenable journals, repository binding, and the node:sqlite guard #138, and the sentence should be trimmed to "they skip" whenever that lands. Flagging it rather than pre-writing a claim that is not true yet.