Skip to content

Commit

Permalink
fix(aspects) - do not try to fetch local aspects when resolving them (#…
Browse files Browse the repository at this point in the history
  • Loading branch information
GiladShoham authored Sep 22, 2024
1 parent 8ee7ee4 commit 1ee78e8
Show file tree
Hide file tree
Showing 3 changed files with 34 additions and 16 deletions.
23 changes: 14 additions & 9 deletions scopes/harmony/aspect-loader/aspect-loader.main.runtime.ts
Original file line number Diff line number Diff line change
Expand Up @@ -813,28 +813,34 @@ export class AspectLoaderMain {
return graph;
}

public async loadAspectFromPath(localAspects: string[]) {
public async loadAspectFromPath(localAspects: string[]): Promise<Record<string, string>> {
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;
}

Expand All @@ -844,8 +850,7 @@ export class AspectLoaderMain {
}

public async resolveLocalAspects(ids: string[], runtime?: string): Promise<AspectDefinition[]> {
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;
Expand Down
25 changes: 19 additions & 6 deletions scopes/workspace/workspace/workspace-aspects-loader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,8 +106,9 @@ export class WorkspaceAspectsLoader {
ids: ${ids.join(', ')}
needed-for: ${neededFor || '<unknown>'}. 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));
Expand Down Expand Up @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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) => {
Expand Down
2 changes: 1 addition & 1 deletion scopes/workspace/workspace/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, string> = {};
filter: Filter;
constructor(
/**
Expand Down

0 comments on commit 1ee78e8

Please sign in to comment.