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(dependency): fix incorrect dependency resolution by favouring latest #143

Merged
merged 1 commit into from
Dec 3, 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
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,38 @@ This has a considerable impact on the developer experience and is one of the mos
An automated depenedency resolver can be used to address the missing dependencies by computing it from the previous packages. This will be tied into a process enhancement in the build command where packages will be expanded with all the dependencies before being requested for build. In addition two addtional commands will provided such as 'shrink' and 'expand'. Shrink could be used by a developer to generate a concise form of sfdx-project.json with all the transitive dependencies removed. Expand as the name suggest exactly does the reverse. This will reduce the verbosity seen in many projects and allows to maintain fidelity with other tools/plugins

## Decision

sfpowerscripts will be utilizing option #3


Option 3 has been chosen as the preferred solution. This approach provides the following benefits:

1. **Maintains Compatibility**: By working with the existing sfdx-project.json format, we ensure compatibility with all SFDX tools and plugins.

2. **Reduces Verbosity**: Developers can maintain a minimal sfdx-project.json with direct dependencies, while the resolver expands it to include all transitive dependencies.

3. **Early Detection**: The resolver helps identify missing or problematic dependencies early in the development cycle.

4. **Strict Dependency Validation**:
- Enforces Salesforce's requirement that package dependencies must be acyclic
- Fails fast when circular dependencies are detected
- Provides clear error messages to help developers resolve dependency issues

5. **Flexible Workflow**: The 'shrink' and 'expand' commands allow developers to switch between concise and complete dependency declarations as needed.

## Consequences

1. **Positive**:
- Reduced maintenance burden for developers
- Early detection of dependency issues
- Strict enforcement of Salesforce's package dependency requirements
- Maintains compatibility with existing tools

2. **Negative**:
- Additional processing step during build
- Build failures when circular dependencies are detected (though this is desired behavior)

3. **Neutral**:
- Teams need to decide whether to commit expanded or shrunk sfdx-project.json

## References

- Issue #855
- [Salesforce Package Development Model](https://developer.salesforce.com/docs/atlas.en-us.sfdx_dev.meta/sfdx_dev/sfdx_dev_dev2gp_plan_pkg_model.htm)
222 changes: 205 additions & 17 deletions src/core/package/dependencies/TransitiveDependencyResolver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,28 @@
import SFPLogger, { LoggerLevel, Logger } from '@flxbl-io/sfp-logger';
import _ from 'lodash';
import UserDefinedExternalDependencyMap from '../../project/UserDefinedExternalDependency';
import semver from 'semver';

export interface DependencyDetail {
version: string;
isDirect: boolean;
contributors: string[];
}

export interface DependencyResolutionDetails {
resolvedDependencies: Map<string, { package: string; versionNumber?: string }[]>;
details: Map<string, { [dependencyName: string]: DependencyDetail }>;
}

export default class TransitiveDependencyResolver {
constructor(private sfdxProjectConfig: any, private logger?: Logger) {}

public async resolveTransitiveDependencies(): Promise<Map<string, { package: string; versionNumber?: string }[]>> {
const result = await this.resolveTransitiveDependenciesWithDetails();
return result.resolvedDependencies;
}

public async resolveTransitiveDependenciesWithDetails(): Promise<DependencyResolutionDetails> {
SFPLogger.log('Validating Project Dependencies...', LoggerLevel.INFO, this.logger);

let clonedProjectConfig = await _.cloneDeep(this.sfdxProjectConfig);
Expand All @@ -17,23 +34,81 @@
pkgWithDependencies,
new UserDefinedExternalDependencyMap().fetchDependencyEntries(clonedProjectConfig)
);
pkgWithDependencies = this.fillDepsTransitively(pkgWithDependencies);

// Track version contributors during resolution
const versionContributors = new Map<string, Map<string, Set<string>>>();
const originalDeps = new Map(pkgWithDependencies);
pkgWithDependencies = this.fillDepsTransitively(pkgWithDependencies, versionContributors);
let sortedPackages = this.topologicalSort(pkgWithDependencies);
let sortedPkgWithDependencies = new Map<string, { package: string; versionNumber?: string }[]>();

sortedPackages.forEach(pkg => {
let dependencies = pkgWithDependencies.get(pkg) || [];
// Remove duplicates
let uniqueDependencies = new Map<string, { package: string; versionNumber?: string }>();
dependencies.forEach(dep => {
uniqueDependencies.set(dep.package, dep);
const existing = uniqueDependencies.get(dep.package);
if (!existing || this.compareVersions(dep.versionNumber, existing.versionNumber) > 0) {
uniqueDependencies.set(dep.package, dep);
}
});
// Sort dependencies according to the topological order
let sortedDependencies = Array.from(uniqueDependencies.values()).sort((a, b) => sortedPackages.indexOf(a.package) - sortedPackages.indexOf(b.package));
let sortedDependencies = Array.from(uniqueDependencies.values())
.sort((a, b) => sortedPackages.indexOf(a.package) - sortedPackages.indexOf(b.package));
sortedPkgWithDependencies.set(pkg, sortedDependencies);
});

return sortedPkgWithDependencies;
// Generate and log dependency details
const details = this.generateDependencyDetails(
sortedPackages,
sortedPkgWithDependencies,
originalDeps,
versionContributors
);

return {
resolvedDependencies: sortedPkgWithDependencies,
details
};
}

private compareVersions(version1?: string, version2?: string): number {
if (!version1 && !version2) return 0;
if (!version1) return -1;
if (!version2) return 1;

// Handle LATEST/NEXT suffixes
const v1HasLatest = version1.endsWith('.LATEST');
const v2HasLatest = version2.endsWith('.LATEST');
const v1HasNext = version1.endsWith('.NEXT');
const v2HasNext = version2.endsWith('.NEXT');

// If one has LATEST and other doesn't, LATEST wins
if ((v1HasLatest || v1HasNext) && !v2HasLatest && !v2HasNext) return 1;
if ((v2HasLatest || v2HasNext) && !v1HasLatest && !v1HasNext) return -1;

// Extract base version (removing LATEST/NEXT if present)
const v1Base = version1.replace(/\.(LATEST|NEXT)$/, '');
const v2Base = version2.replace(/\.(LATEST|NEXT)$/, '');

// Split into version parts
const v1Parts = v1Base.split('.');
const v2Parts = v2Base.split('.');

// Compare first three parts using semver
const v1Semver = v1Parts.slice(0, 3).join('.');
const v2Semver = v2Parts.slice(0, 3).join('.');

const semverCompare = semver.compare(
semver.coerce(v1Semver) || '0.0.0',
semver.coerce(v2Semver) || '0.0.0'
);

if (semverCompare !== 0) return semverCompare;

// If semver parts are equal, compare build numbers (4th part)
const buildNum1 = parseInt(v1Parts[3] || '0');
const buildNum2 = parseInt(v2Parts[3] || '0');

return buildNum1 - buildNum2;
}

private fillDepsWithUserDefinedExternalDependencyMap(
Expand All @@ -48,46 +123,160 @@
return pkgWithDependencies;
}

private generateDependencyDetails(
sortedPackages: string[],
resolvedDeps: Map<string, { package: string; versionNumber?: string }[]>,
originalDeps: Map<string, { package: string; versionNumber?: string }[]>,
versionContributors: Map<string, Map<string, Set<string>>>
): Map<string, { [dependencyName: string]: DependencyDetail }> {
const details = new Map<string, { [dependencyName: string]: DependencyDetail }>();

sortedPackages.forEach(pkg => {
const dependencies = resolvedDeps.get(pkg) || [];

if (dependencies.length > 0) {
SFPLogger.log(
COLOR_HEADER(`\nPackage: ${pkg}`),
LoggerLevel.INFO,
this.logger
);
SFPLogger.log(
COLOR_HEADER('----------------------------------------'),
LoggerLevel.INFO,
this.logger
);
SFPLogger.log(COLOR_HEADER('Dependencies:'), LoggerLevel.INFO, this.logger);

const pkgDetails: { [dependencyName: string]: DependencyDetail } = {};

dependencies.forEach(dep => {
const isDirect = originalDeps.get(pkg)?.some(d =>
d.package === dep.package && d.versionNumber === dep.versionNumber
) || false;

const contributorsSet = versionContributors.get(dep.package)?.get(dep.versionNumber || '') || new Set<string>();
const contributors = Array.from(contributorsSet);

let message = `${dep.package}@${dep.versionNumber || 'unknown'}`;
if (isDirect) {
message += ' (direct dependency)';
} else if (contributors.length > 0) {
message += ` (via ${contributors.join(', ')})`;
}

SFPLogger.log(
COLOR_KEY_MESSAGE(` ${message}`),
LoggerLevel.INFO,
this.logger
);

pkgDetails[dep.package] = {
version: dep.versionNumber || 'unknown',
isDirect,
contributors
};
});

details.set(pkg, pkgDetails);
}
});

return details;
}

private fillDepsTransitively(
pkgWithDependencies: Map<string, { package: string; versionNumber?: string }[]>
pkgWithDependencies: Map<string, { package: string; versionNumber?: string }[]>,
versionContributors: Map<string, Map<string, Set<string>>>
): Map<string, { package: string; versionNumber?: string }[]> {
let dependencyMap = new Map(pkgWithDependencies);
const resolveDependencies = (pkg: string) => {

const resolveDependencies = (pkg: string, chain: Set<string> = new Set()): { package: string; versionNumber?: string }[] => {
SFPLogger.log(
COLOR_HEADER(`fetching dependencies for package:`) + COLOR_KEY_MESSAGE(pkg),
LoggerLevel.TRACE,
this.logger
);

if (chain.has(pkg)) {
const circularChain = Array.from(chain).join(' -> ');
const errorMessage = `Circular dependency detected: ${circularChain} -> ${pkg}. Salesforce does not support circular dependencies between packages.`;
SFPLogger.log(
COLOR_ERROR(errorMessage),
LoggerLevel.ERROR,
this.logger
);
throw new Error(errorMessage);
}

chain.add(pkg);

let dependencies = dependencyMap.get(pkg) || [];
let allDependencies = new Set(dependencies);
let allDependencies = new Map<string, { package: string; versionNumber?: string }>();

// Add direct dependencies
dependencies.forEach(dep => {
const existing = allDependencies.get(dep.package);
if (!existing || this.compareVersions(dep.versionNumber, existing.versionNumber) > 0) {
allDependencies.set(dep.package, dep);

// Track version contributor
if (!versionContributors.has(dep.package)) {
versionContributors.set(dep.package, new Map());
}
const packageVersions = versionContributors.get(dep.package)!;
if (!packageVersions.has(dep.versionNumber || '')) {
packageVersions.set(dep.versionNumber || '', new Set());
}
packageVersions.get(dep.versionNumber || '')!.add(pkg);
}
});

// Add transitive dependencies
dependencies.forEach(dep => {
if (dependencyMap.has(dep.package)) {
let transitiveDeps = resolveDependencies(dep.package);
transitiveDeps.forEach(td => allDependencies.add(td));
let transitiveDeps = resolveDependencies(dep.package, new Set(chain));
transitiveDeps.forEach(td => {
const existing = allDependencies.get(td.package);
if (!existing || this.compareVersions(td.versionNumber, existing.versionNumber) > 0) {
allDependencies.set(td.package, td);

// Track version contributor
if (!versionContributors.has(td.package)) {
versionContributors.set(td.package, new Map());

Check warning on line 245 in src/core/package/dependencies/TransitiveDependencyResolver.ts

View check run for this annotation

Codecov / codecov/patch

src/core/package/dependencies/TransitiveDependencyResolver.ts#L245

Added line #L245 was not covered by tests
}
const packageVersions = versionContributors.get(td.package)!;
if (!packageVersions.has(td.versionNumber || '')) {
packageVersions.set(td.versionNumber || '', new Set());
}
packageVersions.get(td.versionNumber || '')!.add(dep.package);
}
});
}
});
return Array.from(allDependencies);

chain.delete(pkg);
return Array.from(allDependencies.values());
};

for (let pkg of dependencyMap.keys()) {
let resolvedDeps = resolveDependencies(pkg);
dependencyMap.set(pkg, resolvedDeps);
}

return dependencyMap;
}


private swapAndDropArrayElement<T>(arr: T[], i: number, j: number): T[] {
if (i < 0 || i >= arr.length || j < 0 || j >= arr.length) {
return arr;
return arr;

Check warning on line 271 in src/core/package/dependencies/TransitiveDependencyResolver.ts

View check run for this annotation

Codecov / codecov/patch

src/core/package/dependencies/TransitiveDependencyResolver.ts#L271

Added line #L271 was not covered by tests
}

let newArr = [...arr];
[newArr[i], newArr[j]] = [newArr[j], newArr[i]];
return [...newArr.slice(0, j), ...newArr.slice(j + 1)];
}
}

private topologicalSort(
private topologicalSort(
pkgWithDependencies: Map<string, { package: string; versionNumber?: string }[]>
): string[] {
let visited = new Set<string>();
Expand All @@ -108,5 +297,4 @@

return result;
}

}
Loading