Skip to content

Conversation

@transphorm
Copy link
Member

@transphorm transphorm commented Sep 9, 2025

Summary

  • avoid cache busts on test changes
  • cache and reuse built deps in iOS E2E workflow
  • add incremental dependency build helper

Testing

  • yarn workspaces foreach -p -v --topological-dev --since=HEAD run nice --if-present (fails: Unable to resolve path to module '@selfxyz/mobile-sdk-alpha/constants/analytics')
  • yarn lint (fails: Unable to resolve path to module '@selfxyz/mobile-sdk-alpha/constants/analytics')
  • yarn build
  • yarn workspace @selfxyz/contracts build (fails: Couldn't find a script named "hardhat")
  • yarn types (fails: Cannot find module 'dompurify' or its corresponding type declarations)
  • yarn workspace @selfxyz/mobile-sdk-alpha test
  • yarn workspace @selfxyz/mobile-app test

https://chatgpt.com/codex/tasks/task_b_68c06de14878832db6a58c29144c55e4

Summary by CodeRabbit

  • Chores
    • Optimized build pipeline with caching for mobile E2E, reducing redundant dependency builds.
    • Introduced conditional dependency builds that only run when relevant files change.
    • Refined cache key generation to ignore test files, improving cache hit reliability and build speed.

@coderabbitai
Copy link
Contributor

coderabbitai bot commented Sep 9, 2025

Walkthrough

Adds a cache-aware build flow for mobile E2E on iOS, updates cache keys to ignore test files, introduces a conditional dependency-build script, and wires new build:deps scripts in affected packages to the CI workflow.

Changes

Cohort / File(s) Summary
Cache action updates
.github/actions/cache-built-deps/action.yml
Expands hashFiles patterns to exclude test files/dirs when computing cache keys for restore/save steps.
Mobile E2E workflow
.github/workflows/mobile-e2e.yml
Adds “Cache Built Dependencies” step using local action; builds dependencies only on cache miss; updates logging accordingly.
Package scripts
app/package.json, packages/mobile-sdk-alpha/package.json
Replaces/introduces build:deps to invoke node ../..\/scripts/build-deps-if-changed.mjs for targeted packages.
Conditional build script
scripts/build-deps-if-changed.mjs
New script: detects changed paths vs. base ref and selectively builds dependent workspaces (@selfxyz/common, @selfxyz/mobile-sdk-alpha) as needed.

Sequence Diagram(s)

sequenceDiagram
  autonumber
  participant GH as GitHub Actions (iOS E2E)
  participant Cache as cache-built-deps Action
  participant App as @selfxyz/mobile-app
  participant Script as build-deps-if-changed.mjs
  participant WS as Yarn Workspaces

  GH->>Cache: Compute cache key (tests excluded)
  Cache-->>GH: cache-hit = true/false
  alt Cache miss
    GH->>App: run build:deps
    App->>Script: node build-deps-if-changed.mjs @selfxyz/mobile-app
    Script->>Script: git diff baseRef...HEAD
    alt Changes in common/
      Script->>WS: yarn workspace @selfxyz/common build
    else No changes
      Script-->>WS: skip
    end
    alt Changes in packages/mobile-sdk-alpha/
      Script->>WS: yarn workspace @selfxyz/mobile-sdk-alpha build
    else No changes
      Script-->>WS: skip
    end
  else Cache hit
    GH-->>App: skip build
  end
Loading
sequenceDiagram
  autonumber
  participant Dev as Env
  participant Script as build-deps-if-changed.mjs
  participant Git as Git
  participant WS as Yarn Workspaces

  Dev->>Script: workspace arg
  Script->>Git: resolve baseRef (GITHUB_BASE_REF || origin/dev)
  Script->>Git: git diff --name-only baseRef...HEAD
  Git-->>Script: list of changed paths (or empty on error)
  Script->>Script: match changes against watched dirs
  alt Any matches
    Script->>WS: yarn workspace <dep> build (per matched dep)
  else No matches
    Script-->>Dev: Skips all builds
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Suggested reviewers

  • remicolin
  • aaronmgdr

Pre-merge checks (3 passed)

✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title Check ✅ Passed The title "chore: speed up ios e2e builds" is concise, uses a conventional "chore" prefix, and accurately summarizes the primary intent of the changeset (caching and incremental builds to reduce iOS E2E build time).
Docstring Coverage ✅ Passed No functions found in the changes. Docstring coverage check skipped.

Poem

Cache whispers, “hit,” and builds stand down,
On misses, scripts don workman’s crown.
Git points where the changes grew,
Common and SDK line up on cue.
Quiet speed in CI’s lane,
Less to build, more to gain. 🚀

Warning

Review ran into problems

🔥 Problems

Errors were encountered while retrieving linked issues.

Errors (1)
  • Failed to retrieve linked issues from the platform client.
✨ Finishing Touches
  • 📝 Generate Docstrings
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch codex/analyze-changes-for-increased-build-time

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Base automatically changed from temp/reselect-applet to dev September 9, 2025 19:54
@transphorm
Copy link
Member Author

don't think these "fixes" are worth it. closing

@transphorm transphorm closed this Sep 11, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 4

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
.github/actions/cache-built-deps/action.yml (1)

21-30: Bug: saving cache before build guarantees no cache is ever saved

This composite calls actions/cache/save immediately after restore. On a miss, nothing is built yet, so the save step archives empty/nonexistent dist folders. Result: persistent cache misses and slower builds.

Two safe options:

  • Split into two composites (restore/save) and call “save” after the build in the workflow, or
  • Remove the save step here and add a dedicated “Save Built Dependencies” step in the workflow after deps are built.

Minimal fix (remove premature save here):

     - name: Save Built Dependencies
-      if: steps.restore.outputs.cache-hit != 'true'
-      uses: actions/cache/save@v4
-      with:
-        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/**', '!common/**/__tests__/**', '!common/**/*.test.*', '!common/**/*.spec.*', '!packages/mobile-sdk-alpha/**/__tests__/**', '!packages/mobile-sdk-alpha/**/*.test.*', '!packages/mobile-sdk-alpha/**/*.spec.*') }}
+      if: false # moved to workflow post-build; see workflow fix

I can also add a cache-built-deps-save composite mirroring the key logic if you prefer to avoid direct actions/cache in workflows. Want me to push that change?

📜 Review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between f343cbb and 77978e6.

📒 Files selected for processing (5)
  • .github/actions/cache-built-deps/action.yml (2 hunks)
  • .github/workflows/mobile-e2e.yml (1 hunks)
  • app/package.json (1 hunks)
  • packages/mobile-sdk-alpha/package.json (1 hunks)
  • scripts/build-deps-if-changed.mjs (1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
packages/mobile-sdk-alpha/package.json

📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)

packages/mobile-sdk-alpha/package.json: Expose a 'test:build' script in the SDK's package.json that runs build, test, types, and lint
Enable tree shaking for the SDK (e.g., ensure 'sideEffects' is correctly set in package.json and exports are ESM-friendly)

Files:

  • packages/mobile-sdk-alpha/package.json
packages/mobile-sdk-alpha/**/package.json

📄 CodeRabbit inference engine (packages/mobile-sdk-alpha/AGENTS.md)

packages/mobile-sdk-alpha/**/package.json: Ensure package exports are properly configured
Verify package conditions are valid (e.g., exports conditions)

Files:

  • packages/mobile-sdk-alpha/package.json
app/package.json

📄 CodeRabbit inference engine (.cursor/rules/mobile-sdk-migration.mdc)

Expose a 'test:build' script in the app's package.json that builds deps, types, performs bundle analysis, and runs tests

Files:

  • app/package.json
.github/workflows/**/*.{yml,yaml}

📄 CodeRabbit inference engine (AGENTS.md)

.github/workflows/**/*.{yml,yaml}: In GitHub Actions workflows, use shared composite caching actions from .github/actions (cache-yarn, cache-bundler, cache-gradle, cache-pods)
Do not call actions/cache directly; rely on the shared composite caching actions
When using cache actions, optionally pass cache-version (often with GH_CACHE_VERSION and tool version) for stable keys

Files:

  • .github/workflows/mobile-e2e.yml
🧠 Learnings (19)
📓 Common learnings
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-08-24T18:52:25.796Z
Learning: Use yarn scripts: yarn ios/android for builds, yarn test for unit tests, and Fastlane for deployments
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/package.json : Expose a 'test:build' script in the SDK's package.json that runs build, test, types, and lint

Applied to files:

  • packages/mobile-sdk-alpha/package.json
  • app/package.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/package.json : Ensure package exports are properly configured

Applied to files:

  • packages/mobile-sdk-alpha/package.json
  • app/package.json
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/package.json : Enable tree shaking for the SDK (e.g., ensure 'sideEffects' is correctly set in package.json and exports are ESM-friendly)

Applied to files:

  • packages/mobile-sdk-alpha/package.json
  • app/package.json
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to app/package.json : Expose a 'test:build' script in the app's package.json that builds deps, types, performs bundle analysis, and runs tests

Applied to files:

  • packages/mobile-sdk-alpha/package.json
  • app/package.json
  • scripts/build-deps-if-changed.mjs
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/package.json : Verify package conditions are valid (e.g., exports conditions)

Applied to files:

  • packages/mobile-sdk-alpha/package.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/**/*.{ts,tsx} : Avoid introducing circular dependencies

Applied to files:

  • packages/mobile-sdk-alpha/package.json
  • .github/actions/cache-built-deps/action.yml
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/src/index.ts : Re-export new SDK modules via packages/mobile-sdk-alpha/src/index.ts

Applied to files:

  • packages/mobile-sdk-alpha/package.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Use actual imports from selfxyz/mobile-sdk-alpha in tests

Applied to files:

  • packages/mobile-sdk-alpha/package.json
  • app/package.json
  • .github/actions/cache-built-deps/action.yml
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/README.md : Document new/updated SDK modules and usage in packages/mobile-sdk-alpha/README.md

Applied to files:

  • packages/mobile-sdk-alpha/package.json
📚 Learning: 2025-08-29T15:31:15.924Z
Learnt from: CR
PR: selfxyz/self#0
File: packages/mobile-sdk-alpha/AGENTS.md:0-0
Timestamp: 2025-08-29T15:31:15.924Z
Learning: Applies to packages/mobile-sdk-alpha/{**/*.test.{ts,tsx},**/__tests__/**/*.{ts,tsx}} : Do NOT mock selfxyz/mobile-sdk-alpha in tests (avoid jest.mock('selfxyz/mobile-sdk-alpha') and replacing real functions with mocks)

Applied to files:

  • packages/mobile-sdk-alpha/package.json
  • .github/actions/cache-built-deps/action.yml
📚 Learning: 2025-08-24T18:52:25.796Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursorrules:0-0
Timestamp: 2025-08-24T18:52:25.796Z
Learning: Use yarn scripts: yarn ios/android for builds, yarn test for unit tests, and Fastlane for deployments

Applied to files:

  • packages/mobile-sdk-alpha/package.json
  • app/package.json
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/demo/** : Provide an in-SDK lightweight React Native demo under packages/mobile-sdk-alpha/demo/

Applied to files:

  • packages/mobile-sdk-alpha/package.json
📚 Learning: 2025-08-24T18:54:04.809Z
Learnt from: CR
PR: selfxyz/self#0
File: .cursor/rules/mobile-sdk-migration.mdc:0-0
Timestamp: 2025-08-24T18:54:04.809Z
Learning: Applies to packages/mobile-sdk-alpha/samples/** : Create sample applications under packages/mobile-sdk-alpha/samples/ (RN demo and web demo)

Applied to files:

  • packages/mobile-sdk-alpha/package.json
📚 Learning: 2025-08-29T15:29:47.727Z
Learnt from: CR
PR: selfxyz/self#0
File: AGENTS.md:0-0
Timestamp: 2025-08-29T15:29:47.727Z
Learning: Applies to .github/workflows/**/*.{yml,yaml} : In GitHub Actions workflows, use shared composite caching actions from .github/actions (cache-yarn, cache-bundler, cache-gradle, cache-pods)

Applied to files:

  • .github/workflows/mobile-e2e.yml
  • .github/actions/cache-built-deps/action.yml
📚 Learning: 2025-08-29T15:29:47.727Z
Learnt from: CR
PR: selfxyz/self#0
File: AGENTS.md:0-0
Timestamp: 2025-08-29T15:29:47.727Z
Learning: Applies to .github/workflows/**/*.{yml,yaml} : When using cache actions, optionally pass cache-version (often with GH_CACHE_VERSION and tool version) for stable keys

Applied to files:

  • .github/workflows/mobile-e2e.yml
  • .github/actions/cache-built-deps/action.yml
📚 Learning: 2025-08-29T15:29:47.727Z
Learnt from: CR
PR: selfxyz/self#0
File: AGENTS.md:0-0
Timestamp: 2025-08-29T15:29:47.727Z
Learning: Applies to .github/workflows/**/*.{yml,yaml} : Do not call actions/cache directly; rely on the shared composite caching actions

Applied to files:

  • .github/workflows/mobile-e2e.yml
📚 Learning: 2025-08-29T15:30:12.210Z
Learnt from: CR
PR: selfxyz/self#0
File: app/AGENTS.md:0-0
Timestamp: 2025-08-29T15:30:12.210Z
Learning: Verify iOS build succeeds via yarn ios

Applied to files:

  • .github/workflows/mobile-e2e.yml
📚 Learning: 2025-08-29T15:29:47.727Z
Learnt from: CR
PR: selfxyz/self#0
File: AGENTS.md:0-0
Timestamp: 2025-08-29T15:29:47.727Z
Learning: Before PRs, ensure yarn build succeeds for all workspaces

Applied to files:

  • scripts/build-deps-if-changed.mjs
⏰ Context from checks skipped due to timeout of 300000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (3)
  • GitHub Check: e2e-ios
  • GitHub Check: analyze-ios
  • GitHub Check: analyze-android

Comment on lines +244 to 255
- 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
Copy link
Contributor

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.

"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

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.

],
"scripts": {
"build": "rm -rf dist && tsup && yarn postbuild",
"build:deps": "node ../../scripts/build-deps-if-changed.mjs @selfxyz/mobile-sdk-alpha",
Copy link
Contributor

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.

Comment on lines +10 to +22
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 = [];
}
Copy link
Contributor

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

@transphorm transphorm deleted the codex/analyze-changes-for-increased-build-time branch September 12, 2025 23:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants