Adds CI coverage for the existing Detox iOS E2E suites in RNApp, ExpoApp54, and ExpoApp55.#352
Adds CI coverage for the existing Detox iOS E2E suites in RNApp, ExpoApp54, and ExpoApp55.#352alpharius-ck wants to merge 30 commits into
Conversation
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
# Conflicts: # turbo.json
Co-authored-by: Cursor <cursoragent@cursor.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
alpharius-ck
changed the title
hurali97
left a comment
hurali97
left a comment
There was a problem hiding this comment.
Great work. As explained in the comments, we should add e2e testing behavior for a brownfield workflow and not greenfield. Most of the changes in the PR are unnecessary as they try to fix greenfield and while in a real project environment that might makes sense, in case of this repo, we do not need it.
Our main goal should be to verify AppleApp and AndroidApp works in e2e testing with brownfield packages.
| @@ -0,0 +1,10 @@ | |||
| --- | |||
| '@callstack/brownfield-example-shared-tests': minor | |||
There was a problem hiding this comment.
This should not be a part of the changeset.
| function detoxAttrsText(attrs) { | ||
| if (!attrs || typeof attrs !== 'object') { | ||
| return ''; | ||
| } | ||
| const fragment = (o) => | ||
| [o.text, o.value, o.label, o.hint] | ||
| .filter((x) => x != null && String(x).length > 0) | ||
| .join(''); | ||
| if ('elements' in attrs && Array.isArray(attrs.elements)) { | ||
| return attrs.elements.map(fragment).join('').trim(); | ||
| } | ||
| return fragment(attrs).trim(); | ||
| } | ||
|
|
||
| async function assertDetoxTextMatches(nativeElement, pattern) { | ||
| const attrs = await nativeElement.getAttributes(); | ||
| assert.match(detoxAttrsText(attrs).trim(), pattern); | ||
| } |
There was a problem hiding this comment.
These two appears to be duplicated in expoPostMessage.e2e test as well. Perhaps you can extract these to test utils.
| assert.match(detoxAttrsText(attrs).trim(), pattern); | ||
| } | ||
|
|
||
| const ids = { |
There was a problem hiding this comment.
Since you already have defined ids at apps/brownfield-example-shared-tests/src/e2eTestIds.ts I believe we can remove from here
| // Keep compatibility with Expo's default app key. | ||
| AppRegistry.registerComponent('main', () => App); | ||
| AppRegistry.registerComponent('RNApp', () => RNAppRoot); | ||
| AppRegistry.registerComponent('main', () => RNAppRoot); |
There was a problem hiding this comment.
I believe we still want to show ExpoRoot for main entry point
| "ios": "yarn brownfield:package:ios && node ../../packages/cli/dist/main.js codegen && expo run:ios", | ||
| "web": "expo start --web", | ||
| "lint": "expo lint --no-cache", | ||
| "test": "jest --config jest.config.js", | ||
| "prebuild": "expo prebuild", | ||
| "e2e:build:ios": "detox build --configuration ios.sim.debug", | ||
| "e2e:test:ios": "detox test --configuration ios.sim.debug", | ||
| "ci:local:e2e:ios": "bash ../../scripts/ci-local-expo54-ios-e2e.sh", | ||
| "prebuild": "node ../../packages/cli/dist/main.js codegen && expo prebuild", |
There was a problem hiding this comment.
Why are we running the codegen script explicitly? when you run brownfield:package:ios the codegen for brownie and navigation kicks off automatically.
There was a problem hiding this comment.
Let's avoid these changes as explained above, we only test these apps with AppleApp and AndroidApp, not standalone.
| :ccache_enabled => ENV['USE_CCACHE'] == '1' | ||
| ) | ||
|
|
||
| # >>> react-native-brownfield Expo SDK 55+ swift defines >>> |
There was a problem hiding this comment.
If this is for Expo, why are we making these changes for RNApp
There was a problem hiding this comment.
Can u explain why we need these changes? For a brownfield app, we already have everything working. Unless you're running RNApp as a standalone and these changes are required for that?
| return project.findProject(EXPO_PROJECT_LOCATOR) != null | ||
| } | ||
|
|
||
| /** |
There was a problem hiding this comment.
Let's revert these changes as explained above.
There was a problem hiding this comment.
All of the navigation changes should not be tracked. These files are auto generated each time you run package:ios or package:android. Please revert these files and compare with main on what should be tracked
4 participants
Adds CI coverage for the existing Detox iOS E2E suites in RNApp, ExpoApp54, and ExpoApp55.
Introduces a reusable composite action (.github/actions/e2e-ios) that:
Test plan
If a job fails, download the detox-rnapp-ios / detox-expo54-ios / detox-expo55-ios artifact and inspect screenshots/logs
Changes for native files:
BrownfieldStore “not found” fix (RNApp iOS)
This document summarizes the investigation and fixes for the Brownie error when running RNApp on iOS:
AppleApp (brownfield host) worked with the same JS and store schema; only standalone RNApp failed until these changes were applied.
The problem
When running RNApp on iOS, React crashed on the first render of
Counterwith the error above.useStore('BrownfieldStore', …)in JavaScript could not see a store that native code was supposed to have registered.How Brownie works (short)
Brownie shares state between native and React Native through two layers:
BrownieStoreManager— holds stores and backsglobal.__brownieGetStore(what JS uses).Store/StoreManager— optional native-side wrapper;BrownfieldStore.register(...)creates aStoreand calls into the C++ bridge.JavaScript (
@callstack/brownie) reads stores via:That global is installed by
BrownieInstaller::install(), which must run when the Brownie turbo module is loaded under the New Architecture.Native registration must hit the same C++ manager instance that JavaScript uses.
Root causes
There were two separate issues, not one.
1. Duplicate Brownie in RNApp (main RNApp bug)
RNApp linked and embedded
BrownfieldLib.frameworkinto the app. That framework was built withinherit! :completein the Podfile, so it contained a full second copy of Brownie (including its ownBrownieStoreManagersingleton).At runtime:
AppDelegate→BrownfieldStore.register(...)__brownieGetStore→ C++ lookupRegistration and JS were talking to different singletons, so JS always saw “store not found” even though registration appeared to succeed on native.
AppleApp avoids this: it links one
Brownie.xcframeworkand oneBrownfieldLib.xcframeworkseparately, and registers viaimport Browniein the app — not by embedding a BrownfieldLib that re-exports and duplicates Brownie.2. JSI bindings not installed on iOS New Architecture (brownie package bug)
On iOS with
RCT_NEW_ARCH_ENABLED, React installs turbo-module JSI bindings only when the module is a C++TurboModuleWithJSIBindings.BrownieModuleonly implemented the ObjC protocolRCTTurboModuleWithJSIBindings. In bridgeless / New Architecture, that path is not used;dynamic_cast<TurboModuleWithJSIBindings*>on the C++ module fails, soBrownieInstaller::install()never ran andglobal.__brownieGetStorestayed undefined.Android already handled this in
BrownieModule.initialize()viainstallJSIBindingsIfNeeded().This could affect any iOS React Native 0.85+ app using Brownie; RNApp made it obvious because JS hits the store immediately on launch.
What we changed
RNApp — iOS app target
ios/RNApp.xcodeproj/project.pbxprojBrownfieldLibfrom the RNApp app target. TheBrownfieldLibtarget remains forbrownfield package:ios.ios/RNApp/AppDelegate.swiftimport Brownie; register store beforefactory.startReactNative(...)viaBrownieBootstrap.register(...).ios/BrownfieldLib/BrownfieldLib.swift@_exported import Brownie. Only re-exportsReactBrownfield.BrownfieldLibRNApp — Android
android/app/src/main/java/com/rnapp/MainApplication.ktregisterStoreIfNeeded(...)forBrownfieldStorebeforeloadReactNative(this).android/app/build.gradleimplementation(project(":BrownfieldLib"))for generated Kotlin types.@callstack/browniepackagepackages/brownie/ios/BrownieModule.mmBrownieTurboModuleextendingNativeBrownieModuleSpecJSI+TurboModuleWithJSIBindings;getTurboModulereturns it;installJSIBindingsWithRuntimecallsBrownieInstaller::install(runtime).__brownieGetStorenever set on New Architecturepackages/brownie/ios/BrownieModule.hRCTTurboModuleWithJSIBindingsimport from header.packages/brownie/ios/BrownieStore.swiftBrownieBootstrap— registers viaBrownieStoreBridgedirectly (C++ path JS uses).RNApp — tooling
package.jsonios/androidscripts runyarn codegenfirst.Codegen (operational)
From
apps/RNApp:This generates:
packages/brownie/ios/Generated/BrownfieldStore.swift(gitignored)android/BrownfieldLib/.../Generated/BrownfieldStore.ktWhy each fix works
Stop embedding
BrownfieldLibin RNAppRNApp as a standalone React Native app does not need the brownfield packaging framework at runtime. Embedding it pulled in a second Brownie. After removal, there is one
BrownieStoreManagerin the process; registration and JS use the same store map.BrownfieldLibis still built foryarn brownfield:package:ios; it is just not part of the dev app binary anymore.Register before React Native starts
Brownie expects stores to exist before the JS bundle runs components that call
useStore. BothAppDelegate(iOS) andMainApplication(Android) register initial state before starting RN.BrownieTurboModule(library fix)When JS first loads the Brownie turbo module, React calls
TurboModuleWithJSIBindings::installJSIBindingson the C++ module. That runsBrownieInstaller::install, which definesglobal.__brownieGetStore. Without this, JS fails even with a correctly registered native store.BrownieBootstrap(optional)BrownfieldStore.registercreates a SwiftStoreand also talks to the bridge. After the duplicate was removed,BrownfieldStore.registeris sufficient for RNApp again.BrownieBootstrapregisters straight throughBrownieStoreBridgeand documents “use this from AppDelegate before RN starts.” You can use either:No Brownie re-export in
BrownfieldLibPackaged
BrownfieldLib.xcframeworkshould not embed another full Brownie. Host apps (like AppleApp) link Brownie explicitly. That keeps brownfield packaging aligned with AppleApp’s working layout.Required vs optional going forward
Required for RNApp
BrownfieldLibin the RNApp app target.BrownieTurboModulein@callstack/browniefor iOS New Architecture.yarn codegenwhen store schemas change.Optional
BrownieBootstrapvsBrownfieldStore.registerinAppDelegate(equivalent after duplicate fix).yarn codegen &&prepended toios/androidscripts.Unchanged
BrownfieldStore.registerin appinit, separateBrownie+BrownfieldLibxcframeworks — no change needed.End-to-end flow after fixes (RNApp iOS)
sequenceDiagram participant AD as AppDelegate participant BB as Brownie C++ manager participant RN as React Native participant JS as JS useStore AD->>BB: BrownieBootstrap / register store AD->>RN: startReactNative RN->>JS: Load bundle JS->>RN: Require Brownie turbo module RN->>BB: BrownieTurboModule installs __brownieGetStore JS->>BB: __brownieGetStore("BrownfieldStore") BB-->>JS: Host object with stateTakeaway
The error looked like “forgot to register the store,” but RNApp actually had two separate issues:
BrownfieldLib) — why AppleApp worked and RNApp did not.BrownieTurboModule.Removing the duplicate was the decisive fix for RNApp; the turbo module fix is still necessary for correct Brownie behavior on modern React Native iOS.