Feat/repack bundler #13536
Feat/repack bundler #13536
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub. 4 Skipped Deployments
|
Desktop Bundle Checks✅ Previous issues have all been fixed. Mobile Bundle Checks
|
KVNLS
force-pushed
the
fix/circular-dependency-redux
branch
from
1674479 to
f831fd5
Compare
KVNLS
force-pushed
the
feat/repack-bundler
branch
4 times, most recently
from
c7c0873 to
2bc56e5
Compare
KVNLS
force-pushed
the
feat/repack-bundler
branch
5 times, most recently
from
e474f65 to
52cf9e2
Compare
There was a problem hiding this comment.
Pull request overview
This PR migrates Ledger Live Mobile from Metro bundler to Repack (powered by rspack) to achieve faster build times, better monorepo compatibility, and enhanced developer tooling through Rozenite integration.
Changes:
- Replaced Metro bundler with Repack/rspack for significantly faster build times and better pnpm workspace resolution
- Added Rozenite integration for enhanced debugging capabilities (network monitoring, Redux DevTools, navigation inspection, MMKV storage)
- Updated build scripts and iOS/Android configurations to support the new bundler architecture
Reviewed changes
Copilot reviewed 8 out of 10 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| pnpm-lock.yaml | Added @callstack/repack packages, @rspack/core, and @rozenite/repack; removed @react-native/metro-config and @rozenite/metro |
| apps/ledger-live-mobile/rspack.config.mjs | New rspack configuration with TypeScript alias resolution, pnpm workspace support, and Rozenite integration |
| apps/ledger-live-mobile/react-native.config.js | Added Repack rspack commands configuration |
| apps/ledger-live-mobile/package.json | Updated bundle scripts with increased Node memory allocation and added new Repack dependencies |
| apps/ledger-live-mobile/babel.config.js | Added metro-transform-plugins for inline requires and removed styled-components plugin configuration |
| apps/ledger-live-mobile/src/reducers/accounts.ts | Added optional chaining to accountsSelector for safer state access |
| apps/ledger-live-mobile/ios/ledgerlivemobile.xcodeproj/project.pbxproj | Updated React Native bundling script to properly detect CLI path |
| apps/ledger-live-mobile/ios/Podfile.lock | Added callstack-repack pod with dependencies (JWTDecode, SwiftyRSA) |
| apps/ledger-live-mobile/android/app/build.gradle | Added androidx.appcompat:appcompat dependency |
| knip.json | Updated ignored dependencies to reflect bundler migration |
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| * This is important because the `accounts` array is modified at the end almost | ||
| * every account sync, so potentially you could avoid many unnessary and | ||
| * expensive re-renders. | ||
| */ |
There was a problem hiding this comment.
The optional chaining operator (?.) is added to safely access s.accounts.active. While this prevents potential runtime errors, it would be helpful to understand why this defensive coding is necessary. Is there a scenario where s.accounts could be undefined or null during the migration to Repack? If this is a workaround for a bundler-related initialization issue, it should be documented in a comment explaining the context.
| */ | |
| */ | |
| // Note: we use optional chaining on `s.accounts` here because, during app startup | |
| // (e.g. store hydration, state migrations, or Repack/bundler-related initialization), | |
| // the `accounts` slice can briefly be undefined/null before the reducer state is | |
| // fully initialized. This defensive access avoids crashes in those transient states. |
| "@babel/plugin-transform-export-namespace-from", | ||
| "@babel/plugin-transform-class-static-block", | ||
| "@babel/plugin-transform-flow-strip-types", | ||
| ["@babel/plugin-transform-private-methods", { loose: true }], |
There was a problem hiding this comment.
The babel-plugin-styled-components configuration has been removed from babel.config.js. This plugin was previously configured with specific options for SSR, display names, minification, and template literal transpilation. Ensure that styled-components still functions correctly without this configuration, or verify that the Repack bundler handles styled-components transformation internally. If this functionality is no longer needed, consider removing the babel-plugin-styled-components dependency from package.json as well.
| ["@babel/plugin-transform-private-methods", { loose: true }], | |
| ["@babel/plugin-transform-private-methods", { loose: true }], | |
| [ | |
| "babel-plugin-styled-components", | |
| { | |
| ssr: true, | |
| displayName: true, | |
| minify: true, | |
| transpileTemplateLiterals: true, | |
| }, | |
| ], |
| import path from "node:path"; | ||
| import { fileURLToPath } from "node:url"; | ||
| import * as Repack from "@callstack/repack"; | ||
| import { ReanimatedPlugin } from "@callstack/repack-plugin-reanimated"; | ||
| import { ExpoModulesPlugin } from "@callstack/repack-plugin-expo-modules"; | ||
| import { createRequire } from "node:module"; | ||
| import { readFileSync, existsSync, readdirSync } from "node:fs"; | ||
| import { withRozenite } from "@rozenite/repack"; | ||
|
|
||
| const require = createRequire(import.meta.url); | ||
|
|
||
| const __filename = fileURLToPath(import.meta.url); | ||
| const __dirname = path.dirname(__filename); | ||
| const projectRootDir = path.resolve(__dirname, "..", ".."); | ||
|
|
||
| const tsconfig = JSON.parse(readFileSync(path.join(__dirname, "./tsconfig.json"), "utf8")); | ||
| const resolveFromPnpmSubmodule = (...subPath) => { | ||
| const pnpmDir = path.resolve(projectRootDir, "node_modules", ".pnpm"); | ||
| for (const entry of readdirSync(pnpmDir)) { | ||
| const candidate = path.join(pnpmDir, entry, ...subPath); | ||
| if (existsSync(candidate)) return candidate; | ||
| } | ||
| throw new Error(`Cannot resolve pnpm submodule path: ${path.join(...subPath)}`); | ||
| }; | ||
|
|
||
| const removeStarPath = moduleName => moduleName.replace("/*", ""); | ||
| const buildTsAlias = (paths = {}) => | ||
| Object.keys(paths).reduce( | ||
| (acc, moduleName) => ({ | ||
| ...acc, | ||
| [removeStarPath(moduleName)]: path.resolve(__dirname, removeStarPath(paths[moduleName][0])), | ||
| }), | ||
| {}, | ||
| ); | ||
|
|
||
| const nodeModulesPaths = [ | ||
| path.resolve(__dirname, "node_modules"), | ||
| path.resolve(projectRootDir, "node_modules"), | ||
| path.resolve(projectRootDir, "node_modules", ".pnpm"), | ||
| path.resolve(projectRootDir, "node_modules", ".pnpm", "node_modules"), | ||
| "node_modules", | ||
| ]; | ||
|
|
||
| const hermesParserPath = require.resolve("hermes-parser"); | ||
|
|
||
| export default withRozenite( | ||
| Repack.defineRspackConfig(env => { | ||
| const { mode, platform } = env; | ||
| return { | ||
| mode, | ||
| context: __dirname, | ||
| entry: "./index.js", | ||
| resolve: { | ||
| ...Repack.getResolveOptions(platform, { | ||
| enablePackageExports: true, | ||
| preferNativePlatform: true, | ||
| }), | ||
| fullySpecified: false, | ||
| modules: nodeModulesPaths, | ||
| extensions: [ | ||
| `.${platform}.tsx`, | ||
| `.${platform}.ts`, | ||
| `.${platform}.jsx`, | ||
| `.${platform}.js`, | ||
| ".native.tsx", | ||
| ".native.ts", | ||
| ".native.jsx", | ||
| ".native.js", | ||
| ".tsx", | ||
| ".ts", | ||
| ".jsx", | ||
| ".js", | ||
| ".mjs", | ||
| ".cjs", | ||
| ".json", | ||
| "...", | ||
| ], | ||
| enforceExtension: false, | ||
| mainFields: ["react-native", "browser", "main", "module"], | ||
| conditionNames: ["require", "react-native", "browser"], | ||
| alias: { | ||
| ...buildTsAlias(tsconfig.compilerOptions.paths), | ||
| // Packages with malformed exports field (missing "." subpath) - resolve to browser entry | ||
| "@aptos-labs/aptos-client": resolveFromPnpmSubmodule( | ||
| "node_modules", | ||
| "@aptos-labs", | ||
| "aptos-client", | ||
| "dist", | ||
| "browser", | ||
| "index.browser.mjs", | ||
| ), | ||
| "rpc-websockets": resolveFromPnpmSubmodule( | ||
| "node_modules", | ||
| "rpc-websockets", | ||
| "dist", | ||
| "index.browser.mjs", | ||
| ), | ||
| }, | ||
| fallback: { | ||
| ...require("node-libs-react-native"), | ||
| fs: require.resolve("react-native-level-fs"), | ||
| net: require.resolve("react-native-tcp-socket"), | ||
| tls: false, | ||
| child_process: false, | ||
| cluster: false, | ||
| dgram: false, | ||
| dns: false, | ||
| readline: false, | ||
| module: false, | ||
| repl: false, | ||
| vm: false, | ||
| }, | ||
| }, | ||
| module: { | ||
| rules: [ | ||
| { | ||
| test: /\.[cm]?[jt]sx?$/, | ||
| use: { | ||
| loader: "@callstack/repack/babel-loader", | ||
| parallel: true, | ||
| options: { | ||
| hermesParserPath, | ||
| }, | ||
| }, | ||
| resolve: { | ||
| fullySpecified: false, | ||
| }, | ||
| type: "javascript/auto", | ||
| }, | ||
| ...Repack.getAssetTransformRules(), | ||
| ], | ||
| }, | ||
| plugins: [ | ||
| new Repack.RepackPlugin(), | ||
| new ReanimatedPlugin({ | ||
| unstable_disableTransform: true, | ||
| }), | ||
| new ExpoModulesPlugin(), | ||
| ], | ||
| }; | ||
| }), | ||
| { | ||
| enabled: process.env.WITH_ROZENITE === "true", | ||
| include: [ | ||
| "@rozenite/network-activity-plugin", | ||
| "@rozenite/react-navigation-plugin", | ||
| "@rozenite/redux-devtools-plugin", | ||
| "@rozenite/mmkv-plugin", | ||
| ], | ||
| }, | ||
| ); |
There was a problem hiding this comment.
The new rspack.config.mjs configuration file lacks test coverage. Given that this is a critical infrastructure change affecting the entire build and bundling process, it would be beneficial to add integration tests that verify the bundler configuration works correctly for both iOS and Android platforms in development and production modes.
| for (const entry of readdirSync(pnpmDir)) { | ||
| const candidate = path.join(pnpmDir, entry, ...subPath); | ||
| if (existsSync(candidate)) return candidate; | ||
| } | ||
| throw new Error(`Cannot resolve pnpm submodule path: ${path.join(...subPath)}`); |
There was a problem hiding this comment.
The error handling in resolveFromPnpmSubmodule could be improved. Currently, if no matching pnpm submodule is found, it throws an error with the path that was being searched. However, this error message doesn't show which specific entry was attempted. Consider including more debugging information in the error message, such as the pnpm directory being searched or the list of entries that were checked.
| for (const entry of readdirSync(pnpmDir)) { | |
| const candidate = path.join(pnpmDir, entry, ...subPath); | |
| if (existsSync(candidate)) return candidate; | |
| } | |
| throw new Error(`Cannot resolve pnpm submodule path: ${path.join(...subPath)}`); | |
| const entries = readdirSync(pnpmDir); | |
| for (const entry of entries) { | |
| const candidate = path.join(pnpmDir, entry, ...subPath); | |
| if (existsSync(candidate)) return candidate; | |
| } | |
| throw new Error( | |
| `Cannot resolve pnpm submodule path: ${path.join( | |
| ...subPath, | |
| )}; searched in: ${pnpmDir}; checked entries: ${entries.join(", ") || "<none>"}`, | |
| ); |
KVNLS
force-pushed
the
feat/repack-bundler
branch
from
52cf9e2 to
1f40bcb
Compare
|
KVNLS
force-pushed
the
feat/repack-bundler
branch
from
1f40bcb to
f8938ec
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 11 changed files in this pull request and generated 5 comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "@react-native-firebase/remote-config": "22.2.0", | ||
| "@react-native-masked-view/masked-view": "0.2.9", | ||
| "@react-native/assets-registry": "catalog:", | ||
| "@react-native/community-cli-plugin": "0.77.3", |
There was a problem hiding this comment.
@react-native/community-cli-plugin is pinned to 0.77.3 while this app is on the React Native catalog version (0.79.x). This introduces multiple versions of RN CLI/Metro-related packages in the lockfile and risks incompatibilities. Prefer aligning this to the React Native version used in the repo (e.g., via catalog), or remove the explicit dependency if it’s already provided transitively by react-native/@react-native-community/cli.
| "@react-native/community-cli-plugin": "0.77.3", | |
| "@react-native/community-cli-plugin": "catalog:", |
| <LaunchAction | ||
| buildConfiguration = "Debug" | ||
| buildConfiguration = "Staging" | ||
| selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB" |
There was a problem hiding this comment.
The Xcode scheme’s LaunchAction buildConfiguration was changed from Debug to Staging while TestAction remains Debug. This is a significant behavioral change for local Xcode runs and seems unrelated to the bundler migration. If unintentional, revert to Debug; if intentional, consider documenting why Staging should be the default launch configuration and whether TestAction should be updated for consistency.
| androidTestImplementation('com.wix:detox:+') | ||
| implementation 'androidx.appcompat:appcompat:1.1.0' | ||
| // Needed for Detox getAttributes on Android : https://github.com/wix/Detox/issues/3147 | ||
| implementation 'com.google.android.material:material:1.6.0' |
There was a problem hiding this comment.
Pinning androidx.appcompat:appcompat to 1.1.0 is very old and may cause dependency resolution conflicts with other AndroidX libraries (or miss fixes needed on newer SDKs). If this is required for Detox, consider aligning to the version recommended by Detox/RN docs or the rest of the project’s AndroidX versions; otherwise avoid forcing an explicit appcompat version here.
| const resolveFromPnpmSubmodule = (...subPath) => { | ||
| const pnpmDir = path.resolve(projectRootDir, "node_modules", ".pnpm"); | ||
| for (const entry of readdirSync(pnpmDir)) { | ||
| const candidate = path.join(pnpmDir, entry, ...subPath); | ||
| if (existsSync(candidate)) return candidate; | ||
| } | ||
| throw new Error(`Cannot resolve pnpm submodule path: ${path.join(...subPath)}`); | ||
| }; |
There was a problem hiding this comment.
resolveFromPnpmSubmodule scans every entry under node_modules/.pnpm on each config evaluation to find a matching file. This can be noticeably slow in large workspaces and is brittle if the .pnpm directory layout changes. Prefer resolving from the package root deterministically (e.g., resolve the package.json path with require.resolve using nodeModulesPaths, then join to the desired file), and/or cache results to avoid repeated directory scans.
| setupMiddlewares: middlewares => { | ||
| const result = originalSetupMiddlewares(middlewares); | ||
| result.unshift((req, res, next) => { | ||
| if (req.url?.startsWith("/debugger-frontend/")) { |
There was a problem hiding this comment.
withRozeniteUrlFix wraps devServer.setupMiddlewares but only forwards the first parameter to the original handler. If the underlying devServer implementation passes additional arguments (e.g., the devServer instance), this wrapper will drop them and can break the original middleware setup. Forward all arguments when delegating to originalSetupMiddlewares.
KVNLS
force-pushed
the
feat/repack-bundler
branch
from
f8938ec to
59a539e
Compare
KVNLS
force-pushed
the
feat/repack-bundler
branch
from
59a539e to
83b25b7
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 13 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| runOnlyForDeploymentPostprocessing = 0; | ||
| shellPath = /bin/sh; | ||
| shellScript = "export NODE_BINARY=$(command -v node)\n\nset -e\n\nWITH_ENVIRONMENT=\"../node_modules/react-native/scripts/xcode/with-environment.sh\"\n\nif [[ \"$SKIP_JS_BUNDLING\" == \"true\" ]]; then\n REACT_NATIVE_XCODE=\"\"\n echo \"Skipping JS bundle\"\nelse\n echo \"Not skipping JS bundle\"\n REACT_NATIVE_XCODE=\"../node_modules/react-native/scripts/react-native-xcode.sh\"\nfi\n\nBUNDLE_REACT_NATIVE=\"$REACT_NATIVE_XCODE\"\n\n/bin/sh -c \"$WITH_ENVIRONMENT \\\"$BUNDLE_REACT_NATIVE\\\"\"\n"; | ||
| shellScript = "export NODE_BINARY=$(command -v node)\n\nset -e\n\nWITH_ENVIRONMENT=\"../node_modules/react-native/scripts/xcode/with-environment.sh\"\n\nif [[ \"$SKIP_JS_BUNDLING\" == \"true\" ]]; then\n REACT_NATIVE_XCODE=\"\"\n echo \"Skipping JS bundle\"\nelse\n echo \"Not skipping JS bundle\"\n REACT_NATIVE_XCODE=\"../node_modules/react-native/scripts/react-native-xcode.sh\"\nfi\n\nif [[ -f \"$PODS_ROOT/../.xcode.env\" ]]; then\n source \"$PODS_ROOT/../.xcode.env\"\nfi\nif [[ -f \"$PODS_ROOT/../.xcode.env.local\" ]]; then\n source \"$PODS_ROOT/../.xcode.env.local\"\nfi\n\nexport CLI_PATH=\"$(\"$NODE_BINARY\" --print \"require('path').dirname(require.resolve('@react-native-community/cli/package.json')) + '/build/bin.js'\")\"\n\n/bin/sh -c \"$WITH_ENVIRONMENT $REACT_NATIVE_XCODE\"\n"; |
There was a problem hiding this comment.
The new iOS bundle script always invokes with-environment.sh even when SKIP_JS_BUNDLING=true sets REACT_NATIVE_XCODE to an empty string. In that case, /bin/sh -c "$WITH_ENVIRONMENT $REACT_NATIVE_XCODE" will call with-environment.sh without the target script path and can fail the build. Consider short-circuiting the script when SKIP_JS_BUNDLING is true, or only invoking with-environment.sh when REACT_NATIVE_XCODE is non-empty (and keep proper quoting).
KVNLS
force-pushed
the
feat/repack-bundler
branch
from
83b25b7 to
ba876fc
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 13 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const withRozeniteUrlFix = rozeniteConfig => { | ||
| return async env => { | ||
| const config = await rozeniteConfig(env); | ||
| if (!config.devServer?.setupMiddlewares) { | ||
| return config; | ||
| } | ||
|
|
||
| const originalSetupMiddlewares = config.devServer.setupMiddlewares; | ||
|
|
||
| return { | ||
| ...config, | ||
| devServer: { | ||
| ...config.devServer, | ||
| setupMiddlewares: middlewares => { | ||
| const result = originalSetupMiddlewares(middlewares); | ||
| result.unshift((req, res, next) => { | ||
| if (req.url?.startsWith("/debugger-frontend/")) { | ||
| const newUrl = req.url.replace("/debugger-frontend/", "/rozenite/"); | ||
| res.writeHead(302, { Location: newUrl }); | ||
| res.end(); | ||
| return; | ||
| } | ||
| next(); | ||
| }); | ||
|
|
||
| return result; | ||
| }, | ||
| }, |
There was a problem hiding this comment.
withRozeniteUrlFix wraps devServer.setupMiddlewares but changes the function signature to accept only middlewares. webpack-dev-server passes (middlewares, devServer); dropping the second arg can break upstream middleware setup that relies on devServer.
Recommend preserving the signature by using (...args) => { const result = originalSetupMiddlewares(...args); ... }.
| @@ -274,7 +280,7 @@ | |||
| "babel-jest": "catalog:", | |||
| "babel-plugin-styled-components": "2.1.4", | |||
| "babel-plugin-transform-inline-environment-variables": "0.4.4", | |||
There was a problem hiding this comment.
babel-plugin-transform-inline-environment-variables remains in devDependencies, but it was removed from babel.config.js and doesn’t appear to be referenced elsewhere in the repo. Keeping it adds install surface area without effect.
Either re-add it to the Babel plugins list (if still needed) or remove the dependency from this package.
| "babel-plugin-transform-inline-environment-variables": "0.4.4", |
KVNLS
force-pushed
the
feat/repack-bundler
branch
from
79bc3b9 to
c6ca4f4
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 12 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| implementation 'androidx.appcompat:appcompat:1.1.0' | ||
| // Needed for Detox getAttributes on Android : https://github.com/wix/Detox/issues/3147 |
There was a problem hiding this comment.
The newly added androidx.appcompat:appcompat:1.1.0 is very old relative to the project’s compileSdkVersion = 35 and can introduce unnecessary version skew in the AndroidX dependency graph. Also, the Detox-related comment below now reads as if it applies to material, not the added AppCompat line.
If AppCompat is required, prefer aligning its version with the rest of the resolved AndroidX set (or relying on the transitive AppCompat brought by Material, if applicable) and update/move the comment to match the dependency it refers to.
| implementation 'androidx.appcompat:appcompat:1.1.0' | |
| // Needed for Detox getAttributes on Android : https://github.com/wix/Detox/issues/3147 | |
| // Needed for Detox getAttributes on Android : https://github.com/wix/Detox/issues/3147 | |
| implementation 'androidx.appcompat:appcompat:1.6.1' |
| androidTestImplementation('com.wix:detox:+') | ||
| implementation 'androidx.appcompat:appcompat:1.1.0' | ||
| // Needed for Detox getAttributes on Android : https://github.com/wix/Detox/issues/3147 | ||
| implementation 'com.google.android.material:material:1.6.0' | ||
| implementation 'com.facebook.soloader:soloader:0.10.4+' |
There was a problem hiding this comment.
The Gradle dependencies androidTestImplementation('com.wix:detox:+') and implementation 'com.facebook.soloader:soloader:0.10.4+' use dynamic + versions, which means each build may fetch a newer, unvetted artifact and execute its code with access to your CI environment and secrets. This creates a supply-chain risk where a compromised upstream release or registry could silently introduce malicious code into your test or production builds. To reduce this risk, pin these libraries to explicit, vetted versions (or immutable coordinates) instead of using + or other dynamic version ranges.
KVNLS
force-pushed
the
feat/repack-bundler
branch
from
c6ca4f4 to
c5511cc
Compare
|
✅ Checklist
npx changesetwas attached.📝 Description
What
Migrate Ledger Live Mobile from Metro bundler to Repack (powered by rspack) for faster and more flexible bundling.
Why
❓ Context
Build iOS
Build Android
🧐 Checklist for the PR Reviewers