Skip to content

Commit

Permalink
fix(install) - reload envs which moved (fs) during installation proce…
Browse files Browse the repository at this point in the history
…ss (#9111)
  • Loading branch information
GiladShoham authored Aug 12, 2024
1 parent 58cedbf commit 16d2980
Show file tree
Hide file tree
Showing 9 changed files with 135 additions and 17 deletions.
6 changes: 6 additions & 0 deletions scopes/component/component/component-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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;
};

Expand Down
17 changes: 14 additions & 3 deletions scopes/envs/envs/env.plugin.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand Down Expand Up @@ -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,
Expand All @@ -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;
}
}
6 changes: 5 additions & 1 deletion scopes/envs/envs/environments.main.runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}
Expand Down
2 changes: 1 addition & 1 deletion scopes/envs/envs/envs.cmd.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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}`;
}
Expand Down
4 changes: 3 additions & 1 deletion scopes/generator/generator/workspace-generator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 4 additions & 2 deletions scopes/harmony/aspect-loader/plugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,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.
Expand All @@ -21,7 +21,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;
}
}
5 changes: 4 additions & 1 deletion scopes/harmony/aspect-loader/plugins.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
99 changes: 93 additions & 6 deletions scopes/workspace/install/install.main.runtime.ts
Original file line number Diff line number Diff line change
@@ -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';
Expand All @@ -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,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -119,6 +121,8 @@ export class InstallMain {

private wsConfigFiles: WorkspaceConfigFilesMain,

private aspectLoader: AspectLoaderMain,

private app: ApplicationMain,

private generator: GeneratorMain,
Expand All @@ -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
Expand Down Expand Up @@ -237,7 +243,7 @@ export class InstallMain {
addMissingDeps: installMissing,
skipIfExisting: true,
writeConfigFiles: false,
skipPrune: true,
// skipPrune: true,
});
}

Expand Down Expand Up @@ -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();
Expand All @@ -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,
Expand Down Expand Up @@ -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 & {
Expand Down Expand Up @@ -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}`
Expand Down Expand Up @@ -1031,6 +1112,7 @@ export class InstallMain {
IpcEventsAspect,
GeneratorAspect,
WorkspaceConfigFilesAspect,
AspectLoaderAspect,
];

static runtime = MainRuntime;
Expand All @@ -1049,6 +1131,7 @@ export class InstallMain {
ipcEvents,
generator,
wsConfigFiles,
aspectLoader,
]: [
DependencyResolverMain,
Workspace,
Expand All @@ -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) => {
Expand All @@ -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));
Expand Down
7 changes: 5 additions & 2 deletions scopes/workspace/workspace/workspace-aspects-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export class WorkspaceAspectsLoader {
hideMissingModuleError: !!this.workspace.inInstallContext,
ignoreErrors: false,
resolveEnvsFromRoots: this.resolveEnvsFromRoots,
forceLoad: false,
};
const mergedOpts: Required<WorkspaceLoadAspectsOptions> = { ...defaultOpts, ...opts };

Expand All @@ -107,7 +108,10 @@ needed-for: ${neededFor || '<unknown>'}. 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 [];
Expand Down Expand Up @@ -171,7 +175,6 @@ needed-for: ${neededFor || '<unknown>'}. 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`);
Expand Down

0 comments on commit 16d2980

Please sign in to comment.