Skip to content

chore: speed up ios e2e builds #1032

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 7 commits into from
Closed

chore: speed up ios e2e builds #1032

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
6524b43
fix: CLA not supported
seshanthS Sep 9, 2025
c76c972
Merge branch 'dev' into temp/reselect-applet
transphorm Sep 9, 2025
7531dd5
Merge branch 'dev' into temp/reselect-applet
transphorm Sep 9, 2025
0d38d80
fix "yarn android" building
transphorm Sep 9, 2025
f6b6f1c
remove unnecessary commands
transphorm Sep 9, 2025
d7fcbaa
chore: speed up ios e2e builds
transphorm Sep 9, 2025
77978e6
Merge branch 'dev' into codex/analyze-changes-for-increased-build-time
transphorm Sep 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/actions/cache-built-deps/action.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ runs:
path: |
common/dist
packages/mobile-sdk-alpha/dist
key: built-deps-${{ inputs.cache-version }}-${{ hashFiles('common/**/*', 'packages/mobile-sdk-alpha/**/*', '!common/dist/**', '!packages/mobile-sdk-alpha/dist/**') }}
key: built-deps-${{ inputs.cache-version }}-${{ hashFiles('common/**/*', 'packages/mobile-sdk-alpha/**/*', '!common/dist/**', '!packages/mobile-sdk-alpha/dist/**', '!common/**/__tests__/**', '!common/**/*.test.*', '!common/**/*.spec.*', '!packages/mobile-sdk-alpha/**/__tests__/**', '!packages/mobile-sdk-alpha/**/*.test.*', '!packages/mobile-sdk-alpha/**/*.spec.*') }}
fail-on-cache-miss: false
- name: Save Built Dependencies
if: steps.restore.outputs.cache-hit != 'true'
Expand All @@ -27,4 +27,4 @@ runs:
path: |
common/dist
packages/mobile-sdk-alpha/dist
key: built-deps-${{ inputs.cache-version }}-${{ hashFiles('common/**/*', 'packages/mobile-sdk-alpha/**/*', '!common/dist/**', '!packages/mobile-sdk-alpha/dist/**') }}
key: built-deps-${{ inputs.cache-version }}-${{ hashFiles('common/**/*', 'packages/mobile-sdk-alpha/**/*', '!common/dist/**', '!packages/mobile-sdk-alpha/dist/**', '!common/**/__tests__/**', '!common/**/*.test.*', '!common/**/*.spec.*', '!packages/mobile-sdk-alpha/**/__tests__/**', '!packages/mobile-sdk-alpha/**/*.test.*', '!packages/mobile-sdk-alpha/**/*.spec.*') }}
10 changes: 8 additions & 2 deletions .github/workflows/mobile-e2e.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -241,9 +241,15 @@ jobs:
echo "📱 Verifying iOS Runtime availability..."
echo "Available iOS runtimes:"
xcrun simctl list runtimes | grep iOS
- name: Build dependencies (outside main flow)
- name: Cache Built Dependencies
id: built-deps
uses: ./.github/actions/cache-built-deps
with:
cache-version: ${{ env.GH_CACHE_VERSION }}-${{ env.NODE_VERSION_SANITIZED }}
- name: Build dependencies (cache miss)
if: steps.built-deps.outputs.cache-hit != 'true'
run: |
echo "Building dependencies..."
echo "Cache miss for built dependencies. Building now..."
yarn workspace @selfxyz/mobile-app run build:deps --silent || { echo "❌ Dependency build failed"; exit 1; }
echo "✅ Dependencies built successfully"
- name: Install iOS dependencies
Comment on lines +244 to 255
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Cache logic incomplete: restore happens, but nothing saves post-build

You invoke the composite (which currently saves too early). Move the save to after the “Build dependencies (cache miss)” step so future runs hit the cache.

Apply:

       - name: Cache Built Dependencies
         id: built-deps
         uses: ./.github/actions/cache-built-deps
         with:
           cache-version: ${{ env.GH_CACHE_VERSION }}-${{ env.NODE_VERSION_SANITIZED }}
       - name: Build dependencies (cache miss)
         if: steps.built-deps.outputs.cache-hit != 'true'
         run: |
           echo "Cache miss for built dependencies. Building now..."
           yarn workspace @selfxyz/mobile-app run build:deps --silent || { echo "❌ Dependency build failed"; exit 1; }
           echo "✅ Dependencies built successfully"
+      - name: Save Built Dependencies
+        if: steps.built-deps.outputs.cache-hit != 'true'
+        uses: actions/cache/save@v4
+        with:
+          path: |
+            common/dist
+            packages/mobile-sdk-alpha/dist
+          key: built-deps-${{ env.GH_CACHE_VERSION }}-${{ env.NODE_VERSION_SANITIZED }}-${{ hashFiles('common/**/*', 'packages/mobile-sdk-alpha/**/*', '!common/dist/**', '!packages/mobile-sdk-alpha/dist/**', '!common/**/__tests__/**', '!common/**/*.test.*', '!common/**/*.spec.*', '!packages/mobile-sdk-alpha/**/__tests__/**', '!packages/mobile-sdk-alpha/**/*.test.*', '!packages/mobile-sdk-alpha/**/*.spec.*') }}

Optional: switch to a dedicated .github/actions/cache-built-deps-save to avoid direct actions/cache usage in workflows, matching your guidelines. I can add that if you want.

Committable suggestion skipped: line range outside the PR's diff.

Expand Down
2 changes: 1 addition & 1 deletion app/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"analyze:tree-shaking:web": "yarn web:build && node ./scripts/analyze-tree-shaking.cjs web",
"android": "yarn build:deps && react-native run-android",
"android:ci": "./scripts/mobile-ci-build-android.sh",
"build:deps": "yarn workspaces foreach --from @selfxyz/mobile-app --topological --recursive run build",
"build:deps": "node ../scripts/build-deps-if-changed.mjs @selfxyz/mobile-app",
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

On cache miss this may skip needed builds; see script’s shallow-fetch issue

Because build-deps-if-changed.mjs treats diff failures as “no changes,” a cache miss can still lead to no deps being built and downstream iOS build failures. Either adopt the script fix (fetch base or build-all on diff error) or, in CI, build deps unconditionally on cache miss (see workflow comment).

After applying the script fix, please verify no false skips:

🏁 Script executed:

#!/bin/bash
# Should print "Building @selfxyz/common..." or "@selfxyz/mobile-sdk-alpha..." at least once when cache is empty
node ./scripts/build-deps-if-changed.mjs @selfxyz/mobile-app || true

Length of output: 281

Don't treat git-diff failures as "no changes" — make the script build on diff error or fetch the base before diff

Observed: running node ./scripts/build-deps-if-changed.mjs @selfxyz/mobile-app produced "fatal: ambiguous argument 'origin/dev...HEAD'" then "Skipping @selfxyz/*; no changes" — this false skip on cache miss will break downstream iOS builds.

  • Fix scripts/build-deps-if-changed.mjs: fetch the base ref before running git diff (git fetch origin ) OR, if git-diff returns non-zero, assume changes and build all deps instead of skipping.
  • Alternatively, make CI run app's build:deps unconditionally on a cache miss.
  • Verify by clearing the cache and running: node ./scripts/build-deps-if-changed.mjs @selfxyz/mobile-app — expect at least one "Building @selfxyz/..." line.

Locations: app/package.json (build:deps) and scripts/build-deps-if-changed.mjs.

🤖 Prompt for AI Agents 
In app/package.json line 13 and scripts/build-deps-if-changed.mjs (around where
git diff is run), the current script treats git-diff failures as "no changes"
causing false skips; update the script to first ensure the base ref is available
(run git fetch origin <base-branch> or fetch origin for the PR base) before
running git diff, and change the git-diff error handling so any non-zero exit
from git diff is treated as “changes detected” (i.e., fall back to building the
requested @selfxyz/* deps) rather than skipping; alternatively, add a CI-safe
flag to app/package.json build:deps to force unconditional builds on cache-miss
— implement the fetch-and-fallback behavior in the script and verify by clearing
cache and running node ./scripts/build-deps-if-changed.mjs @selfxyz/mobile-app
to observe at least one "Building @selfxyz/..." line.

"bump-version:major": "npm version major && yarn sync-versions",
"bump-version:minor": "npm version minor && yarn sync-versions",
"bump-version:patch": "npm version patch && yarn sync-versions",
Expand Down
1 change: 1 addition & 0 deletions packages/mobile-sdk-alpha/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@
],
"scripts": {
"build": "rm -rf dist && tsup && yarn postbuild",
"build:deps": "node ../../scripts/build-deps-if-changed.mjs @selfxyz/mobile-sdk-alpha",
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Unblock CI: type/module resolution for subpath “constants/analytics” fails pre-build

Lint/types fail with “Unable to resolve …/constants/analytics” when dist isn’t built. Exports point types to dist, which doesn’t exist during dev/lint. Provide TS fallback via typesVersions to source so editors/linters resolve without a build.

Apply:

   "exports": {
@@
   },
+  "typesVersions": {
+    "*": {
+      "constants/analytics": ["src/constants/analytics.ts"],
+      "stores": ["src/stores.ts"],
+      "browser": ["src/browser.ts"],
+      "*": ["src/*"]
+    }
+  },

Optional follow-up: add a react-native condition in exports to prefer RN entry over browser in Metro.

Committable suggestion skipped: line range outside the PR's diff.

🤖 Prompt for AI Agents 
packages/mobile-sdk-alpha/package.json around line 50: the package currently
points TypeScript types to dist which doesn't exist during dev/lint causing
resolution errors for imports like “constants/analytics”; add a typesVersions
entry that maps the problematic subpath(s) to the source TS files so
editors/linters resolve without a build (for example map "constants/*" to
"src/constants/*" or provide a wildcard fallback that prefers src/*), then save
package.json and re-run lint; optionally, update the exports map to include a
"react-native" condition that points to the RN entry to prefer Metro over the
"browser" entry.

"postbuild": "node ./scripts/postBuild.mjs",
"demo:android": "yarn workspace demo-app android",
"demo:ios": "yarn workspace demo-app ios",
Expand Down
41 changes: 41 additions & 0 deletions scripts/build-deps-if-changed.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,41 @@
#!/usr/bin/env node
import { execSync } from 'node:child_process';

const workspace = process.argv[2];
if (!workspace) {
console.error('Usage: build-deps-if-changed <workspace>');
process.exit(1);
}

const baseRef = process.env.GITHUB_BASE_REF
? `origin/${process.env.GITHUB_BASE_REF}`
: 'origin/dev';
let diff;
try {
diff = execSync(`git diff --name-only ${baseRef}...HEAD`, {
encoding: 'utf8',
})
.split('\n')
.filter(Boolean);
} catch {
diff = [];
}
Comment on lines +10 to +22
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Fix: shallow-fetch makes diff empty; builds get silently skipped on cache miss

actions/checkout defaults to fetch-depth: 1, so origin/${GITHUB_BASE_REF} often isn’t present. Your git diff throws, you catch-and-suppress, and diff=[] causes all deps to be skipped. That will break iOS builds when the built-deps cache misses. Fail closed or fetch the base ref; if still unavailable, build all.

Apply:

-const baseRef = process.env.GITHUB_BASE_REF
-  ? `origin/${process.env.GITHUB_BASE_REF}`
-  : 'origin/dev';
-let diff;
-try {
-  diff = execSync(`git diff --name-only ${baseRef}...HEAD`, {
-    encoding: 'utf8',
-  })
-    .split('\n')
-    .filter(Boolean);
-} catch {
-  diff = [];
-}
+const baseBranch = process.env.GITHUB_BASE_REF || 'dev';
+const baseRef = `origin/${baseBranch}`;
+let diff = null;
+try {
+  // Ensure base ref exists for shallow clones
+  try {
+    execSync(`git rev-parse --verify ${baseRef}`, { stdio: 'ignore' });
+  } catch {
+    execSync(`git fetch --no-tags --depth=1 origin ${baseBranch}:${baseRef}`, { stdio: 'ignore' });
+  }
+  diff = execSync(`git diff --name-only ${baseRef}...HEAD`, { encoding: 'utf8' })
+    .split('\n')
+    .filter(Boolean);
+} catch (e) {
+  console.warn(`[build-deps-if-changed] Unable to compute diff against ${baseRef}: ${e?.message ?? e}`);
+  // Leave diff=null to trigger safe default: build all deps
+}
@@
-for (const [pkg, dir] of deps) {
-  const changed = diff.some(f => f.startsWith(dir));
+for (const [pkg, dir] of deps) {
+  const changed = diff ? diff.some(f => f.startsWith(dir)) : true;
   if (changed) {
     console.log(`Building ${pkg}...`);
     execSync(`yarn workspace ${pkg} build`, { stdio: 'inherit' });
   } else {
     console.log(`Skipping ${pkg}; no changes`);
   }
 }

Also applies to: 33-41


const deps =
{
'@selfxyz/mobile-app': [
['@selfxyz/common', 'common/'],
['@selfxyz/mobile-sdk-alpha', 'packages/mobile-sdk-alpha/'],
],
'@selfxyz/mobile-sdk-alpha': [['@selfxyz/common', 'common/']],
}[workspace] || [];

for (const [pkg, dir] of deps) {
const changed = diff.some(f => f.startsWith(dir));
if (changed) {
console.log(`Building ${pkg}...`);
execSync(`yarn workspace ${pkg} build`, { stdio: 'inherit' });
} else {
console.log(`Skipping ${pkg}; no changes`);
}
}
Loading