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

aspects loading fixes #6308

Draft
wants to merge 23 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
ed7540b
do not add extension with empty data and config to the component exte…
GiladShoham Aug 14, 2022
e551b25
add non core aspects with id rather than name when they only register…
GiladShoham Aug 14, 2022
4d40913
fix aspect id when aspect only register data on component load but no…
GiladShoham Aug 15, 2022
a26cab7
fix find extension - compare also with new extension id
GiladShoham Aug 15, 2022
fc1e91d
increase test timeout
GiladShoham Aug 16, 2022
e8da91f
Merge branch 'master' into aspects-loading-fixes
GiladShoham Aug 16, 2022
cd3842e
trying to increase timeouts in more places
GiladShoham Aug 16, 2022
4d133ae
remove wrong timeout
GiladShoham Aug 16, 2022
fac069e
moving jest timeout to base config
GiladShoham Aug 16, 2022
36995aa
types
GiladShoham Aug 16, 2022
b889cc9
increase timeout
GiladShoham Aug 16, 2022
61dfbff
tsignore for this.timeout in legacy tests to prevent conflicts betwee…
GiladShoham Aug 16, 2022
570d19c
Merge branch 'master' into aspects-loading-fixes
GiladShoham Aug 17, 2022
3017a59
Merge branch 'master' into aspects-loading-fixes
GiladShoham Aug 31, 2022
4e914f4
Merge branch 'master' into aspects-loading-fixes
GiladShoham Sep 1, 2022
d78d059
do not add data to self on component load
GiladShoham Sep 1, 2022
5c41731
improve legacy extension list find function
GiladShoham Sep 1, 2022
60ef1a4
ignore version when upserting extension data
GiladShoham Sep 1, 2022
9c609c4
Merge branch 'master' into aspects-loading-fixes
GiladShoham Sep 1, 2022
7d06e4b
aspect get fixes
GiladShoham Sep 1, 2022
3759430
Merge branch 'aspects-loading-fixes' of https://github.com/teambit/bi…
GiladShoham Sep 1, 2022
3b2d470
do save aspect data on aspect itself if it's a core aspect (to make s…
GiladShoham Sep 4, 2022
997915d
Merge branch 'master' into aspects-loading-fixes
GiladShoham Sep 4, 2022
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
2 changes: 1 addition & 1 deletion scopes/component/component/aspect-list.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ export class AspectList {
*/
get(id: string): AspectEntry | undefined {
return this.entries.find((entry) => {
return entry.legacy.stringId === id || entry.id.toStringWithoutVersion() === id;
return entry.legacy.stringId === id || entry.id.toString() === id || entry.id.toStringWithoutVersion() === id;
});
}

Expand Down
2 changes: 1 addition & 1 deletion scopes/react/react/jest/jest.base.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,6 @@ module.exports = {
'^.+\\.module\\.(css|sass|scss)$': require.resolve('identity-obj-proxy'),
},
moduleFileExtensions: ['web.js', 'js', 'web.ts', 'ts', 'web.tsx', 'tsx', 'json', 'web.jsx', 'jsx', 'node'],
testTimeout: 30000, // @todo remove this once mocha-tester is ready and aspect-api testing are using it.
testTimeout: 100000, // @todo remove this once mocha-tester is ready and aspect-api testing are using it.
// watchPlugins: [require.resolve('jest-watch-typeahead/filename'), require.resolve('jest-watch-typeahead/testname')],
};
6 changes: 3 additions & 3 deletions scopes/workspace/workspace/build-graph-from-fs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,15 +70,15 @@ export class GraphFromFsBuilder {
return this.graph;
}

private getAllDepsUnfiltered(component: Component) {
private getAllDepsUnfiltered(component: Component): BitIds {
return component.getAllDependenciesIds().difference(this.ignoreIds);
}

private async getAllDepsFiltered(component: Component): Promise<BitIds> {
const depsWithoutIgnore = this.getAllDepsUnfiltered(component);
const shouldLoadFunc = this.shouldLoadItsDeps;
if (!shouldLoadFunc) return depsWithoutIgnore;
const deps = await mapSeries(depsWithoutIgnore, async (depId) => {
const deps = await mapSeries(depsWithoutIgnore, async (depId: BitId) => {
const shouldLoad = await shouldLoadFunc(depId);
if (!shouldLoad) this.ignoreIds.push(depId);
return shouldLoad ? depId : null;
Expand Down Expand Up @@ -111,7 +111,7 @@ export class GraphFromFsBuilder {
const allIds = await this.getAllDepsFiltered(component);

const allDependencies = await this.loadManyComponents(allIds, idStr);
Object.entries(component.depsIdsGroupedByType).forEach(([depType, depsIds]) => {
Object.entries(component.depsIdsGroupedByType).forEach(([depType, depsIds]: [string, BitIds]) => {
depsIds.forEach((depId) => {
if (this.ignoreIds.has(depId)) return;
if (!this.graph.hasNode(depId.toString())) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import mapSeries from 'p-map-series';
import { compact, fromPairs, uniq } from 'lodash';
import ConsumerComponent from '@teambit/legacy/dist/consumer/component';
import { MissingBitMapComponent } from '@teambit/legacy/dist/consumer/bit-map/exceptions';
import { AspectLoaderMain } from '@teambit/aspect-loader';
import { getLatestVersionNumber } from '@teambit/legacy/dist/utils';
import { IssuesClasses } from '@teambit/component-issues';
import { ComponentNotFound } from '@teambit/legacy/dist/scope/exceptions';
Expand All @@ -25,7 +26,8 @@ export class WorkspaceComponentLoader {
private workspace: Workspace,
private logger: Logger,
private dependencyResolver: DependencyResolverMain,
private envs: EnvsMain
private envs: EnvsMain,
private aspectLoader: AspectLoaderMain
) {
this.componentsCache = createInMemoryCache({ maxSize: getMaxSizeForComponents() });
}
Expand Down Expand Up @@ -230,7 +232,12 @@ export class WorkspaceComponentLoader {
const entries = this.workspace.onComponentLoadSlot.toArray();
const promises = entries.map(async ([extension, onLoad]) => {
const data = await onLoad(component, loadOpts);
return this.upsertExtensionData(component, extension, data);
const compId = component.id.toString();
const compIdWithoutVersion = component.id.toStringWithoutVersion();
if ((compId !== extension && compIdWithoutVersion !== extension) || (this.aspectLoader.isCoreAspect(extension))) {
return this.upsertExtensionData(component, extension, data);
}
return undefined;
});

// Special load events which runs from the workspace but should run from the correct aspect
Expand Down Expand Up @@ -262,18 +269,28 @@ export class WorkspaceComponentLoader {
}

private async upsertExtensionData(component: Component, extension: string, data: any) {
const existingExtension = component.state.config.extensions.findExtension(extension);
const existingExtension = component.state.config.extensions.findExtension(extension, true);
if (existingExtension && data) {
// Only merge top level of extension data
Object.assign(existingExtension.data, data);
return;
}
component.state.config.extensions.push(await this.getDataEntry(extension, data));
if (data){
const dataEntry = await this.getDataEntry(extension, data);
const cloned = dataEntry.clone();
if (cloned.extensionId) {
const compId = await this.workspace.resolveComponentId(cloned.extensionId);
cloned.extensionId = compId._legacy;
cloned.newExtensionId = compId;
}
component.state.config.extensions.push(cloned);
}
}

private async getDataEntry(extension: string, data: { [key: string]: any }): Promise<ExtensionDataEntry> {
private async getDataEntry(extensionId: string, data: { [key: string]: any }): Promise<ExtensionDataEntry> {
// TODO: @gilad we need to refactor the extension data entry api.
return new ExtensionDataEntry(undefined, undefined, extension, undefined, data);
const entry = ExtensionDataEntry.create(extensionId, undefined, data);
return entry;
}
}

Expand Down
9 changes: 6 additions & 3 deletions scopes/workspace/workspace/workspace.ts
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ export class Workspace implements ComponentFactory {

// TODO: refactor - prefer to avoid code inside the constructor.
this.owner = this.config?.defaultOwner;
this.componentLoader = new WorkspaceComponentLoader(this, logger, dependencyResolver, envs);
this.componentLoader = new WorkspaceComponentLoader(this, logger, dependencyResolver, envs, aspectLoader);
this.validateConfig();
this.bitMap = new BitMap(this.consumer.bitMap, this.consumer);
// memoize this method to improve performance.
Expand Down Expand Up @@ -1140,7 +1140,7 @@ the following envs are used in this workspace: ${availableEnvs.join(', ')}`);
extensionsToMerge.push({ origin, extensions: extensionDataListFiltered, extraData });

loadedExtensionIds.push(
...compact(extensionDataListFiltered.map((e) => e.extensionId?.toStringWithoutVersion()))
...compact(extensionDataListFiltered.map((e: ExtensionDataEntry) => e.extensionId?.toStringWithoutVersion()))
);
};
const setDataListAsSpecific = (extensions: ExtensionDataList) => {
Expand Down Expand Up @@ -1242,7 +1242,10 @@ the following envs are used in this workspace: ${availableEnvs.join(', ')}`);
return componentStatus.modified === true;
}

private filterEnvsFromExtensionsIfNeeded(extensionDataList: ExtensionDataList, envWasFoundPreviously: boolean) {
private filterEnvsFromExtensionsIfNeeded(
extensionDataList: ExtensionDataList,
envWasFoundPreviously: boolean
): { extensionDataListFiltered: ExtensionDataList; envIsCurrentlySet: boolean } {
const envAspect = extensionDataList.findExtension(EnvsAspect.id);
const envFromEnvsAspect = envAspect?.config.env;
const [envsNotFromEnvsAspect, nonEnvs] = partition(extensionDataList, (ext) =>
Expand Down
23 changes: 20 additions & 3 deletions src/consumer/config/extension-data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,16 @@ export class ExtensionDataEntry {
R.clone(this.data)
);
}

static create(extensionId: string, config?: any, data?: any): ExtensionDataEntry {
// eslint-disable-next-line @typescript-eslint/no-use-before-define
const isCore = ExtensionDataList.coreExtensionsNames.has(extensionId);
if (!isCore) {
const parsedId = BitId.parse(extensionId, true);
return new ExtensionDataEntry(undefined, parsedId, undefined, config, data);
}
return new ExtensionDataEntry(undefined, undefined, extensionId, config, data);
}
}

export class ExtensionDataList extends Array<ExtensionDataEntry> {
Expand Down Expand Up @@ -146,20 +156,27 @@ export class ExtensionDataList extends Array<ExtensionDataEntry> {
}

findExtension(extensionId: string, ignoreVersion = false, ignoreScope = false): ExtensionDataEntry | undefined {
const extensionIdWithoutVersion = ignoreVersion ? extensionId.split('@')[0] : extensionId;
if (ExtensionDataList.coreExtensionsNames.has(extensionId)) {
return this.findCoreExtension(extensionId);
}
return this.find((extEntry) => {
if (ignoreVersion && ignoreScope) {
return extEntry.extensionId?.toStringWithoutScopeAndVersion() === extensionId;
return (
extEntry.extensionId?.toStringWithoutScopeAndVersion() === extensionId ||
extEntry.extensionId?.toStringWithoutScopeAndVersion() === extensionIdWithoutVersion ||
extEntry.newExtensionId?.toStringWithoutVersion() === extensionIdWithoutVersion
);
}
if (ignoreVersion) {
return extEntry.extensionId?.toStringWithoutVersion() === extensionId;
return extEntry.extensionId?.toStringWithoutVersion() === extensionId ||
extEntry.extensionId?.toStringWithoutVersion() === extensionIdWithoutVersion ||
extEntry.newExtensionId?.toStringWithoutVersion() === extensionIdWithoutVersion
}
if (ignoreScope) {
return extEntry.extensionId?.toStringWithoutScope() === extensionId;
}
return extEntry.stringId === extensionId;
return extEntry.stringId === extensionId || extEntry.newExtensionId?.toString() === extensionId;
});
}

Expand Down