From d7e1c0d51abedd47f0451009ec404f800ceba939 Mon Sep 17 00:00:00 2001 From: Krzysztof Magiera Date: Mon, 4 Nov 2024 14:33:22 +0100 Subject: [PATCH] Remove stalePods checks from dependency manager to fix rebuild issues (#684) In #457, we added a concept of stale pods flag, that was used to mark whether pod install command should run as a consequence of node_modules. Since that time, I got bitten by this setting causing unexpected rebuilds to the point where some projects I'm testing run native build every single time despite no change in fingerprint. In general I don't like the fact we are adding additional state to facilitate checks that seemingly can be done offline. From my testing, I'm now a bit confused as to what was the purpose of this flag as #457 lacks a proper description and we also don't have any inline comments about this flag and related code. Therefore I'm assuming that the purpose of it was that we trigger pod installation if we had to reinstall node_modules. However, this still seem to not be necessary, as fingerprint seems to deal with this type of scenarios well. Specifically, when we add new native dependencies that get installed in node installation phase, fingerprint will detect that and trigger native rebuild. On top of that, this mechanism, regardless of whether I guessed the original reason for it, is there to prevent from stale builds being used. However, it still appears like it is much easier for the users to recover from issues that may arise from stale native builds than to recover from persistent rebuilds taking many minutes. In the former scenario, all you need to do is to trigger "clean build" reload. In the latter, there's no way for the user to debug that, and I experienced that myself. Fixes # (issue) 1. Test expo-router app, add native dependency, test again 2. I also tested this on reanimated repo example apps with yarn and that's where I've been experiencing the described issue of consistent rebuilds. --- .../src/dependency/DependencyManager.ts | 19 +------------------ 1 file changed, 1 insertion(+), 18 deletions(-) diff --git a/packages/vscode-extension/src/dependency/DependencyManager.ts b/packages/vscode-extension/src/dependency/DependencyManager.ts index b495dc3f5..4ab97b944 100644 --- a/packages/vscode-extension/src/dependency/DependencyManager.ts +++ b/packages/vscode-extension/src/dependency/DependencyManager.ts @@ -6,7 +6,7 @@ import semver, { SemVer } from "semver"; import { Logger } from "../Logger"; import { EMULATOR_BINARY } from "../devices/AndroidEmulatorDevice"; import { command, lineReader } from "../utilities/subprocess"; -import { extensionContext, getAppRootFolder } from "../utilities/extensionContext"; +import { getAppRootFolder } from "../utilities/extensionContext"; import { getIosSourceDir } from "../builders/buildIOS"; import { isExpoGoProject } from "../builders/expoGo"; import { @@ -27,11 +27,8 @@ import { CancelToken } from "../builders/cancelToken"; import { getAndroidSourceDir } from "../builders/buildAndroid"; import { Platform } from "../utilities/platform"; -const STALE_PODS = "stalePods"; - export class DependencyManager implements Disposable, DependencyManagerInterface { // React Native prepares build scripts based on node_modules, we need to reinstall pods if they change - private stalePods = extensionContext.workspaceState.get(STALE_PODS) ?? false; private eventEmitter = new EventEmitter(); private packageManagerInternal: PackageManagerInfo | undefined; @@ -140,8 +137,6 @@ export class DependencyManager implements Disposable, DependencyManagerInterface return false; } - await this.setStalePodsAsync(true); - this.emitEvent("nodeModules", { status: "installing", isOptional: false }); // all managers support the `install` command @@ -183,17 +178,10 @@ export class DependencyManager implements Disposable, DependencyManagerInterface return; } - await this.setStalePodsAsync(false); - this.emitEvent("pods", { status: "installed", isOptional: false }); Logger.debug("Project pods installed"); } - private async setStalePodsAsync(stale: boolean) { - this.stalePods = stale; - await extensionContext.workspaceState.update(STALE_PODS, stale); - } - private async getPackageManager() { if (!this.packageManagerInternal) { this.packageManagerInternal = await resolvePackageManager(); @@ -273,11 +261,6 @@ export class DependencyManager implements Disposable, DependencyManagerInterface return true; } - if (requiresNativeBuild && this.stalePods) { - this.emitEvent("pods", { status: "notInstalled", isOptional: false }); - return false; - } - const appRootFolder = getAppRootFolder(); const iosDirPath = getIosSourceDir(appRootFolder);