From 3962bc35d754ee7f259bacaea17259874a232d2d Mon Sep 17 00:00:00 2001 From: Gilad Shoham Date: Sun, 22 Sep 2024 15:45:29 +0300 Subject: [PATCH] fix(aspects) - do not try to fetch local aspects when resolving them --- .../aspect-loader.main.runtime.ts | 23 ++++++++++------- .../workspace/workspace-aspects-loader.ts | 25 ++++++++++++++----- scopes/workspace/workspace/workspace.ts | 2 +- 3 files changed, 34 insertions(+), 16 deletions(-) diff --git a/scopes/harmony/aspect-loader/aspect-loader.main.runtime.ts b/scopes/harmony/aspect-loader/aspect-loader.main.runtime.ts index 228f29939a71..b1bb9ec69f05 100644 --- a/scopes/harmony/aspect-loader/aspect-loader.main.runtime.ts +++ b/scopes/harmony/aspect-loader/aspect-loader.main.runtime.ts @@ -813,28 +813,34 @@ export class AspectLoaderMain { return graph; } - public async loadAspectFromPath(localAspects: string[]) { + public async loadAspectFromPath(localAspects: string[]): Promise> { + const res = {}; const dirPaths = this.parseLocalAspect(localAspects); - const manifests = dirPaths.map((dirPath) => { + const manifests = dirPaths.map(([dirPath, localAspect]) => { const scopeRuntime = this.findRuntime(dirPath, 'scope'); if (scopeRuntime) { // eslint-disable-next-line global-require, import/no-dynamic-require const module = require(join(dirPath, 'dist', scopeRuntime)); - return module.default || module; + const manifest = module.default || module; + res[manifest.id] = localAspect; + return manifest; } // eslint-disable-next-line global-require, import/no-dynamic-require const module = require(dirPath); - return module.default || module; + const manifest = module.default || module; + res[manifest.id] = localAspect; + return manifest; }); await this.loadExtensionsByManifests(manifests, undefined, { throwOnError: true }); + return res; } private parseLocalAspect(localAspects: string[]) { - const dirPaths = localAspects.map((localAspect) => resolve(localAspect.replace('file://', ''))); - const nonExistsDirPaths = dirPaths.filter((path) => !existsSync(path)); + const dirPaths = localAspects.map((localAspect) => [resolve(localAspect.replace('file://', '')), localAspect]); + const nonExistsDirPaths = dirPaths.filter(([path]) => !existsSync(path)); nonExistsDirPaths.forEach((path) => this.logger.warn(`no such file or directory: ${path}`)); - const existsDirPaths = dirPaths.filter((path) => existsSync(path)); + const existsDirPaths = dirPaths.filter(([path]) => existsSync(path)); return existsDirPaths; } @@ -844,8 +850,7 @@ export class AspectLoaderMain { } public async resolveLocalAspects(ids: string[], runtime?: string): Promise { - const dirs = this.parseLocalAspect(ids); - + const dirs = this.parseLocalAspect(ids).map(([dir]) => dir); return dirs.map((dir) => { const srcRuntimeManifest = runtime ? this.findRuntime(dir, runtime) : undefined; const srcAspectFilePath = runtime ? this.findAspectFile(dir) : undefined; diff --git a/scopes/workspace/workspace/workspace-aspects-loader.ts b/scopes/workspace/workspace/workspace-aspects-loader.ts index d5d3e88286eb..eecc144bdf8a 100644 --- a/scopes/workspace/workspace/workspace-aspects-loader.ts +++ b/scopes/workspace/workspace/workspace-aspects-loader.ts @@ -106,8 +106,9 @@ export class WorkspaceAspectsLoader { ids: ${ids.join(', ')} needed-for: ${neededFor || ''}. using opts: ${JSON.stringify(mergedOpts, null, 2)}`); const [localAspects, nonLocalAspects] = partition(ids, (id) => id.startsWith('file:')); - this.workspace.localAspects = localAspects; - await this.aspectLoader.loadAspectFromPath(this.workspace.localAspects); + const localAspectsMap = await this.aspectLoader.loadAspectFromPath(localAspects); + this.workspace.localAspects = localAspectsMap; + let notLoadedIds = nonLocalAspects; if (!mergedOpts.forceLoad) { notLoadedIds = nonLocalAspects.filter((id) => !this.aspectLoader.isAspectLoaded(id)); @@ -268,12 +269,21 @@ your workspace.jsonc has this component-id set. you might want to remove/change }; const mergedOpts = { ...defaultOpts, ...opts }; const idsToResolve = componentIds ? componentIds.map((id) => id.toString()) : this.harmony.extensionsIds; + const workspaceLocalAspectsIds = Object.keys(this.workspace.localAspects); + const [localAspectsIds, nonLocalAspectsIds] = partition(idsToResolve, (id) => + workspaceLocalAspectsIds.includes(id) + ); + + const localDefs = await this.aspectLoader.resolveLocalAspects( + localAspectsIds.map((id) => this.workspace.localAspects[id]), + runtimeName + ); const coreAspectsIds = this.aspectLoader.getCoreAspectIds(); const configuredAspects = this.aspectLoader.getConfiguredAspects(); // it's possible that componentIds are core-aspects that got version for some reason, remove the version to // correctly filter them out later. - const userAspectsIds: string[] = componentIds - ? componentIds.filter((id) => !coreAspectsIds.includes(id.toStringWithoutVersion())).map((id) => id.toString()) + const userAspectsIds: string[] = nonLocalAspectsIds + ? nonLocalAspectsIds.filter((id) => !coreAspectsIds.includes(id.split('@')[0])).map((id) => id.toString()) : difference(this.harmony.extensionsIds, coreAspectsIds); const rootAspectsIds: string[] = difference(configuredAspects, coreAspectsIds); const componentIdsToResolve = await this.workspace.resolveMultipleComponentIds(userAspectsIds); @@ -295,7 +305,7 @@ your workspace.jsonc has this component-id set. you might want to remove/change ); const idsToFilter = idsToResolve.map((idStr) => ComponentID.fromString(idStr)); - const targetDefs = wsAspectDefs.concat(coreAspectDefs); + const targetDefs = wsAspectDefs.concat(coreAspectDefs).concat(localDefs); const finalDefs = this.aspectLoader.filterAspectDefs(targetDefs, idsToFilter, runtimeName, mergedOpts); return finalDefs; @@ -381,7 +391,10 @@ your workspace.jsonc has this component-id set. you might want to remove/change return coreAspect.runtimePath; }); } - const localResolved = await this.aspectLoader.resolveLocalAspects(this.workspace.localAspects, runtimeName); + const localResolved = await this.aspectLoader.resolveLocalAspects( + Object.keys(this.workspace.localAspects), + runtimeName + ); const allDefsExceptLocal = [...wsAspectDefs, ...coreAspectDefs, ...scopeAspectsDefs, ...installedAspectsDefs]; const withoutLocalAspects = allDefsExceptLocal.filter((aspectId) => { return !localResolved.find((localAspect) => { diff --git a/scopes/workspace/workspace/workspace.ts b/scopes/workspace/workspace/workspace.ts index 8af5b4d66ad9..9e2f6d9dafab 100644 --- a/scopes/workspace/workspace/workspace.ts +++ b/scopes/workspace/workspace/workspace.ts @@ -180,7 +180,7 @@ export class Workspace implements ComponentFactory { * They are used in webpack configuration to only track changes from these paths inside `node_modules` */ private componentPathsRegExps: RegExp[] = []; - localAspects: string[] = []; + localAspects: Record = {}; filter: Filter; constructor( /**