Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(install) - reload envs which moved (fs) during installation process #9111

Merged
merged 2 commits into from
Aug 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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