From 350bf44ac9fd49a4f69881c497a9b9e3d4b961f5 Mon Sep 17 00:00:00 2001 From: Gilad Shoham Date: Mon, 12 Aug 2024 12:06:24 +0300 Subject: [PATCH 1/2] fix(install) - reload envs which moved (fs) during installation process --- .../component/component/component-factory.ts | 6 ++ scopes/envs/envs/env.plugin.ts | 15 ++- scopes/envs/envs/environments.main.runtime.ts | 6 +- scopes/envs/envs/envs.cmd.ts | 2 +- .../generator/workspace-generator.ts | 4 +- scopes/harmony/aspect-loader/plugin.ts | 7 +- scopes/harmony/aspect-loader/plugins.ts | 5 +- .../workspace/install/install.main.runtime.ts | 99 +++++++++++++++++-- .../workspace/workspace-aspects-loader.ts | 7 +- 9 files changed, 135 insertions(+), 16 deletions(-) diff --git a/scopes/component/component/component-factory.ts b/scopes/component/component/component-factory.ts index 711e6daebebd..207c577c7aee 100644 --- a/scopes/component/component/component-factory.ts +++ b/scopes/component/component/component-factory.ts @@ -37,6 +37,12 @@ export type LoadAspectsOptions = { method will print/throw an error if a required module is missing or if any other error occurs during the loading of aspects. */ ignoreErrors?: boolean; + + /** + * Force load the aspect from the host, even if it's already loaded. + */ + forceLoad?: boolean; + [key: string]: any; }; diff --git a/scopes/envs/envs/env.plugin.ts b/scopes/envs/envs/env.plugin.ts index 66498b442911..afa0dba06b8b 100644 --- a/scopes/envs/envs/env.plugin.ts +++ b/scopes/envs/envs/env.plugin.ts @@ -43,6 +43,8 @@ export class EnvPlugin implements PluginDefinition { ...transformers, name: env.name, icon: env.icon, + __path: env.__path, + __resolvedPath: env.__resolvedPath, __getDescriptor: async () => { return { type: env.type || env.name, @@ -52,9 +54,18 @@ export class EnvPlugin implements PluginDefinition { }; } - register(object: any, aspect: Aspect) { + register(object: any, aspect: { id: string }) { const env = this.transformToLegacyEnv(aspect.id, object); if (!env) return undefined; - return this.envSlot.register(env); + // This is required when we call it manually and the aspect id fn return the wrong + // id + // We call the set directly because when we call it manually during install + // the aspect id fn return the wrong id + // Please do not change this without consulting @GiladShoham + // This manual call from install is required to make sure we re-load the envs + // when they move to another location in the node_modules + // during process is still running (like during bit new, bit switch, bit server) + this.envSlot.map.set(aspect.id, env); + return; } } diff --git a/scopes/envs/envs/environments.main.runtime.ts b/scopes/envs/envs/environments.main.runtime.ts index 54e33b40af77..83683b6e6803 100644 --- a/scopes/envs/envs/environments.main.runtime.ts +++ b/scopes/envs/envs/environments.main.runtime.ts @@ -678,10 +678,14 @@ export class EnvsMain { return Boolean(this.getAllEnvsConfiguredOnComponent(component).length); } - getAllRegisteredEnvs(): string[] { + getAllRegisteredEnvsIds(): string[] { return this.envSlot.toArray().map((envData) => envData[0]); } + getAllRegisteredEnvs(): Environment[] { + return this.envSlot.toArray().map((envData) => envData[1]); + } + getAllRegisteredEnvJsoncCustomizers(): EnvJsoncMergeCustomizer[] { return this.envJsoncMergeCustomizerSlot.toArray().map((customizerEntry) => customizerEntry[1]); } diff --git a/scopes/envs/envs/envs.cmd.ts b/scopes/envs/envs/envs.cmd.ts index e304e6dc1eac..0ce23cdfa832 100644 --- a/scopes/envs/envs/envs.cmd.ts +++ b/scopes/envs/envs/envs.cmd.ts @@ -16,7 +16,7 @@ export class ListEnvsCmd implements Command { constructor(private envs: EnvsMain, private componentAspect: ComponentMain) {} async report() { - const allEnvs = this.envs.getAllRegisteredEnvs().join('\n'); + const allEnvs = this.envs.getAllRegisteredEnvsIds().join('\n'); const title = chalk.green('the following envs are available in the workspace:'); return `${title}\n${allEnvs}`; } diff --git a/scopes/generator/generator/workspace-generator.ts b/scopes/generator/generator/workspace-generator.ts index 0cb68cbb22d7..61601b72c85f 100644 --- a/scopes/generator/generator/workspace-generator.ts +++ b/scopes/generator/generator/workspace-generator.ts @@ -79,9 +79,11 @@ export class WorkspaceGenerator { copyPeerToRuntimeOnRoot: true, copyPeerToRuntimeOnComponents: false, updateExisting: false, + // This is not needed anymore since PR: + // keep it here for a while to make sure it doesn't break anything // skip pruning here to prevent cases which it caused an error about // tsconfig not found because the env location was changed - skipPrune: true, + // skipPrune: true, }); // compile the components again now that we have the dependencies installed diff --git a/scopes/harmony/aspect-loader/plugin.ts b/scopes/harmony/aspect-loader/plugin.ts index 5456eb02a297..b73de8b5b6cf 100644 --- a/scopes/harmony/aspect-loader/plugin.ts +++ b/scopes/harmony/aspect-loader/plugin.ts @@ -1,3 +1,4 @@ +import fs from 'fs-extra'; import { Aspect } from '@teambit/harmony'; import { PluginDefinition } from './plugin-definition'; @@ -5,7 +6,7 @@ export class Plugin { constructor(readonly def: PluginDefinition, readonly path: string) {} // consider adding a more abstract type here to allow users to ask for dependencies. - private _instance: undefined | unknown; + private _instance: undefined | any; /** * determines whether the plugin supports a certain runtime. @@ -21,7 +22,9 @@ export class Plugin { require() { // eslint-disable-next-line global-require, import/no-dynamic-require - this._instance = require(this.path).default; + this._instance = require(this.path).default as any; + this._instance.__path = this.path; + this._instance.__resolvedPath = require.resolve(this.path); return this._instance; } } diff --git a/scopes/harmony/aspect-loader/plugins.ts b/scopes/harmony/aspect-loader/plugins.ts index 3e2423d3eabb..0b6745c96d32 100644 --- a/scopes/harmony/aspect-loader/plugins.ts +++ b/scopes/harmony/aspect-loader/plugins.ts @@ -58,7 +58,10 @@ export class Plugins { async loadModule(path: string) { NativeCompileCache.uninstall(); const module = await esmLoader(path); - return module.default; + const defaultModule = module.default; + defaultModule.__path = path; + defaultModule.__resolvedPath = require.resolve(path); + return defaultModule; } async registerPluginWithTryCatch(plugin: Plugin, aspect: Aspect) { diff --git a/scopes/workspace/install/install.main.runtime.ts b/scopes/workspace/install/install.main.runtime.ts index adb846946031..5312f6b6f54f 100644 --- a/scopes/workspace/install/install.main.runtime.ts +++ b/scopes/workspace/install/install.main.runtime.ts @@ -1,3 +1,4 @@ +import pFilter from 'p-filter'; import fs, { pathExists } from 'fs-extra'; import path from 'path'; import { getRootComponentDir, linkPkgsToRootComponents } from '@teambit/workspace.root-components'; @@ -14,7 +15,7 @@ import { VariantsMain, Patterns, VariantsAspect } from '@teambit/variants'; import { Component, ComponentID, ComponentMap } from '@teambit/component'; import { createLinks } from '@teambit/dependencies.fs.linked-dependencies'; import pMapSeries from 'p-map-series'; -import { Slot, SlotRegistry } from '@teambit/harmony'; +import { Harmony, Slot, SlotRegistry } from '@teambit/harmony'; import { CodemodResult, linkToNodeModulesWithCodemod, @@ -52,6 +53,7 @@ import { LinkCommand } from './link'; import InstallCmd from './install.cmd'; import UninstallCmd from './uninstall.cmd'; import UpdateCmd from './update.cmd'; +import AspectLoaderAspect, { AspectLoaderMain } from '@teambit/aspect-loader'; export type WorkspaceLinkOptions = LinkingOptions & { rootPolicy?: WorkspacePolicy; @@ -119,6 +121,8 @@ export class InstallMain { private wsConfigFiles: WorkspaceConfigFilesMain, + private aspectLoader: AspectLoaderMain, + private app: ApplicationMain, private generator: GeneratorMain, @@ -129,7 +133,9 @@ export class InstallMain { private postInstallSlot: PostInstallSlot, - private ipcEvents: IpcEventsMain + private ipcEvents: IpcEventsMain, + + private harmony: Harmony ) {} /** * Install dependencies for all components in the workspace @@ -237,7 +243,7 @@ export class InstallMain { addMissingDeps: installMissing, skipIfExisting: true, writeConfigFiles: false, - skipPrune: true, + // skipPrune: true, }); } @@ -358,6 +364,7 @@ export class InstallMain { ); let cacheCleared = false; await this.linkCodemods(compDirMap); + await this.reloadMovedEnvs(); const shouldClearCacheOnInstall = this.shouldClearCacheOnInstall(); if (options?.compile ?? true) { const compileStartTime = process.hrtime(); @@ -371,6 +378,7 @@ export class InstallMain { cacheCleared = true; } await this.compiler.compileOnWorkspace([], { initiator: CompilationInitiator.Install }); + // Right now we don't need to load extensions/execute load slot at this point // await this.compiler.compileOnWorkspace([], { initiator: CompilationInitiator.Install }, undefined, { // executeLoadSlot: true, @@ -416,6 +424,78 @@ export class InstallMain { return nonLoadedEnvs.length > 0; } + /** + * This function is very important to fix some issues that might happen during the installation process. + * The case is the following: + * during/before the installation process we load some envs from their bit.env files + * this contains code like: + * protected tsconfigPath = require.resolve('./config/tsconfig.json'); + * protected eslintConfigPath = require.resolve('./config/eslintrc.cjs'); + * When we load that file, we calculated the resolved path, and it's stored in the env + * object instance. + * then later on during the install we move the env to another location (like bit roots) + * which points to a .pnpm folder with some peers, that changed during the install + * then when we take this env object and call write ws config for example + * or compile + * we use that resolved path to calculate the final tsconfig + * however that file is no longer exists which result in an error + * This function will check if an env folder doesn't exist anymore, and will re-load it + * from its new location. + * This usually happen when we have install running in the middle of the process followed + * by other bit ops. + * examples: + * bit new - which might run few installs then other ops. + * bit switch - which might run few installs then other ops, and potentially change the + * peer deps during the install. + * bit server (vscode plugin) - which keep the process always live, so any install ops + * that change the location, will cause the vscode plugin/bit server to crash later. + * @returns + */ + private async reloadMovedEnvs() { + const allEnvs = this.envs.getAllRegisteredEnvs(); + const movedEnvs = await pFilter(allEnvs, async (env) => { + if (!env.__path) return false; + const regularPathExists = await pathExists(env.__path); + const resolvedPathExists = await pathExists(env.__resolvedPath); + return !regularPathExists || !resolvedPathExists; + }); + const idsToLoad = movedEnvs.map((env) => env.id); + // const envPlugin = this.envs.getEnvPlugin(); + + if (idsToLoad.length && this.workspace) { + const componentIdsToLoad = idsToLoad.map((id) => ComponentID.fromString(id)); + const aspects = await this.workspace.resolveAspects(undefined, componentIdsToLoad, { + requestedOnly: true, + excludeCore: true, + throwOnError: false, + // Theoretically we should use skipDeps here, but according to implementation at the moment + // it will lead to plugins not load, and we need them to be loaded. + // This is a bug in the flow and should be fixed. + // skipDeps: true, + }); + const loadedPlugins = compact( + await Promise.all( + aspects.map((aspectDef) => { + const localPath = aspectDef.aspectPath; + const component = aspectDef.component; + if (!component) return undefined; + const plugins = this.aspectLoader.getPlugins(component, localPath); + if (plugins.has()) { + return plugins.load(MainRuntime.name); + } + }) + ) + ); + await Promise.all( + loadedPlugins.map((plugin) => { + const runtime = plugin.getRuntime(MainRuntime); + return runtime?.provider(undefined, undefined, undefined, this.harmony); + }) + ); + } + return movedEnvs; + } + private async _getComponentsManifestsAndRootPolicy( installer: DependencyInstaller, options: GetComponentsAndManifestsOptions & { @@ -472,6 +552,7 @@ export class InstallMain { dedupe: true, throw: false, }); + if (err) { this.logger.consoleFailure( `failed generating workspace config files, please run "bit ws-config write" manually. error: ${err.message}` @@ -1031,6 +1112,7 @@ export class InstallMain { IpcEventsAspect, GeneratorAspect, WorkspaceConfigFilesAspect, + AspectLoaderAspect, ]; static runtime = MainRuntime; @@ -1049,6 +1131,7 @@ export class InstallMain { ipcEvents, generator, wsConfigFiles, + aspectLoader, ]: [ DependencyResolverMain, Workspace, @@ -1061,10 +1144,12 @@ export class InstallMain { ApplicationMain, IpcEventsMain, GeneratorMain, - WorkspaceConfigFilesMain + WorkspaceConfigFilesMain, + AspectLoaderMain ], _, - [preLinkSlot, preInstallSlot, postInstallSlot]: [PreLinkSlot, PreInstallSlot, PostInstallSlot] + [preLinkSlot, preInstallSlot, postInstallSlot]: [PreLinkSlot, PreInstallSlot, PostInstallSlot], + harmony: Harmony ) { const logger = loggerExt.createLogger(InstallAspect.id); ipcEvents.registerGotEventSlot(async (eventName) => { @@ -1082,12 +1167,14 @@ export class InstallMain { compiler, envs, wsConfigFiles, + aspectLoader, app, generator, preLinkSlot, preInstallSlot, postInstallSlot, - ipcEvents + ipcEvents, + harmony ); if (issues) { issues.registerAddComponentsIssues(installExt.addDuplicateComponentAndPackageIssue.bind(installExt)); diff --git a/scopes/workspace/workspace/workspace-aspects-loader.ts b/scopes/workspace/workspace/workspace-aspects-loader.ts index b54225ec7f4e..d5d3e88286eb 100644 --- a/scopes/workspace/workspace/workspace-aspects-loader.ts +++ b/scopes/workspace/workspace/workspace-aspects-loader.ts @@ -94,6 +94,7 @@ export class WorkspaceAspectsLoader { hideMissingModuleError: !!this.workspace.inInstallContext, ignoreErrors: false, resolveEnvsFromRoots: this.resolveEnvsFromRoots, + forceLoad: false, }; const mergedOpts: Required = { ...defaultOpts, ...opts }; @@ -107,7 +108,10 @@ needed-for: ${neededFor || ''}. using opts: ${JSON.stringify(mergedOpts const [localAspects, nonLocalAspects] = partition(ids, (id) => id.startsWith('file:')); this.workspace.localAspects = localAspects; await this.aspectLoader.loadAspectFromPath(this.workspace.localAspects); - const notLoadedIds = nonLocalAspects.filter((id) => !this.aspectLoader.isAspectLoaded(id)); + let notLoadedIds = nonLocalAspects; + if (!mergedOpts.forceLoad) { + notLoadedIds = nonLocalAspects.filter((id) => !this.aspectLoader.isAspectLoaded(id)); + } if (!notLoadedIds.length) { this.logger.profile(`[${callId}] workspace.loadAspects`); return []; @@ -171,7 +175,6 @@ needed-for: ${neededFor || ''}. using opts: ${JSON.stringify(mergedOpts throwOnError, opts.runSubscribers ); - await this.aspectLoader.loadExtensionsByManifests(pluginsManifests, undefined, { throwOnError }); const manifestIds = manifests.map((manifest) => manifest.id); this.logger.debug(`${loggerPrefix} finish loading aspects`); From 3818c8046767e7057c369334a2107661676fb1e7 Mon Sep 17 00:00:00 2001 From: Gilad Shoham Date: Mon, 12 Aug 2024 12:26:31 +0300 Subject: [PATCH 2/2] linting --- scopes/envs/envs/env.plugin.ts | 2 +- scopes/harmony/aspect-loader/plugin.ts | 1 - scopes/workspace/install/install.main.runtime.ts | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/scopes/envs/envs/env.plugin.ts b/scopes/envs/envs/env.plugin.ts index afa0dba06b8b..2a25f67249de 100644 --- a/scopes/envs/envs/env.plugin.ts +++ b/scopes/envs/envs/env.plugin.ts @@ -1,5 +1,5 @@ import { PluginDefinition } from '@teambit/aspect-loader'; -import { Aspect, Harmony } from '@teambit/harmony'; +import { Harmony } from '@teambit/harmony'; import { ComponentID } from '@teambit/component'; import { WorkerMain } from '@teambit/worker'; import { MainRuntime } from '@teambit/cli'; diff --git a/scopes/harmony/aspect-loader/plugin.ts b/scopes/harmony/aspect-loader/plugin.ts index b73de8b5b6cf..3d6a2adf4c29 100644 --- a/scopes/harmony/aspect-loader/plugin.ts +++ b/scopes/harmony/aspect-loader/plugin.ts @@ -1,4 +1,3 @@ -import fs from 'fs-extra'; import { Aspect } from '@teambit/harmony'; import { PluginDefinition } from './plugin-definition'; diff --git a/scopes/workspace/install/install.main.runtime.ts b/scopes/workspace/install/install.main.runtime.ts index 5312f6b6f54f..8e4daa0ae638 100644 --- a/scopes/workspace/install/install.main.runtime.ts +++ b/scopes/workspace/install/install.main.runtime.ts @@ -53,7 +53,7 @@ import { LinkCommand } from './link'; import InstallCmd from './install.cmd'; import UninstallCmd from './uninstall.cmd'; import UpdateCmd from './update.cmd'; -import AspectLoaderAspect, { AspectLoaderMain } from '@teambit/aspect-loader'; +import { AspectLoaderAspect, AspectLoaderMain } from '@teambit/aspect-loader'; export type WorkspaceLinkOptions = LinkingOptions & { rootPolicy?: WorkspacePolicy;