diff --git a/decision records/build/001-transitive-dependency-resolver-solution.md b/decision records/build/001-transitive-dependency-resolver-solution.md index cdfafbf89..dae041193 100644 --- a/decision records/build/001-transitive-dependency-resolver-solution.md +++ b/decision records/build/001-transitive-dependency-resolver-solution.md @@ -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) diff --git a/src/core/package/dependencies/TransitiveDependencyResolver.ts b/src/core/package/dependencies/TransitiveDependencyResolver.ts index 0ef6020b0..188952327 100644 --- a/src/core/package/dependencies/TransitiveDependencyResolver.ts +++ b/src/core/package/dependencies/TransitiveDependencyResolver.ts @@ -3,11 +3,28 @@ import { COLOR_HEADER, COLOR_KEY_MESSAGE, COLOR_SUCCESS, COLOR_ERROR } from '@fl 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; + details: Map; +} export default class TransitiveDependencyResolver { constructor(private sfdxProjectConfig: any, private logger?: Logger) {} public async resolveTransitiveDependencies(): Promise> { + const result = await this.resolveTransitiveDependenciesWithDetails(); + return result.resolvedDependencies; + } + + public async resolveTransitiveDependenciesWithDetails(): Promise { SFPLogger.log('Validating Project Dependencies...', LoggerLevel.INFO, this.logger); let clonedProjectConfig = await _.cloneDeep(this.sfdxProjectConfig); @@ -17,23 +34,81 @@ export default class TransitiveDependencyResolver { pkgWithDependencies, new UserDefinedExternalDependencyMap().fetchDependencyEntries(clonedProjectConfig) ); - pkgWithDependencies = this.fillDepsTransitively(pkgWithDependencies); + + // Track version contributors during resolution + const versionContributors = new Map>>(); + const originalDeps = new Map(pkgWithDependencies); + pkgWithDependencies = this.fillDepsTransitively(pkgWithDependencies, versionContributors); let sortedPackages = this.topologicalSort(pkgWithDependencies); let sortedPkgWithDependencies = new Map(); sortedPackages.forEach(pkg => { let dependencies = pkgWithDependencies.get(pkg) || []; - // Remove duplicates let uniqueDependencies = new Map(); 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( @@ -48,46 +123,160 @@ export default class TransitiveDependencyResolver { return pkgWithDependencies; } + private generateDependencyDetails( + sortedPackages: string[], + resolvedDeps: Map, + originalDeps: Map, + versionContributors: Map>> + ): Map { + const details = new Map(); + + 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(); + 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 + pkgWithDependencies: Map, + versionContributors: Map>> ): Map { let dependencyMap = new Map(pkgWithDependencies); - const resolveDependencies = (pkg: string) => { + + const resolveDependencies = (pkg: string, chain: Set = 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(); + + // 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()); + } + 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(arr: T[], i: number, j: number): T[] { if (i < 0 || i >= arr.length || j < 0 || j >= arr.length) { - return arr; + return arr; } 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[] { let visited = new Set(); @@ -108,5 +297,4 @@ export default class TransitiveDependencyResolver { return result; } - } diff --git a/tests/core/package/dependencies/TransitiveDependencyResolver.test.ts b/tests/core/package/dependencies/TransitiveDependencyResolver.test.ts index 64536f6ac..7c25f5834 100644 --- a/tests/core/package/dependencies/TransitiveDependencyResolver.test.ts +++ b/tests/core/package/dependencies/TransitiveDependencyResolver.test.ts @@ -82,7 +82,7 @@ describe("Given a TransitiveDependencyResolver", () => { const resolvedDependencies = await transitiveDependencyResolver.resolveTransitiveDependencies(); let dependencies = resolvedDependencies.get('quote-management'); - expect(dependencies?.find(dependency => dependency.package === "core")?.versionNumber).toBe("1.0.0.LATEST"); + expect(dependencies?.find(dependency => dependency.package === "core")?.versionNumber).toBe("1.2.0.LATEST"); }); @@ -101,6 +101,362 @@ describe("Given a TransitiveDependencyResolver", () => { }); + it("should resolve with a higher version of a given package if a higher version is specified", async () => { + // Setup project config with three packages and their dependencies + const complexProjectConfig = { + packageDirectories: [ + { + package: "package-a", + versionNumber: "1.1.0.NEXT", + dependencies: [] + }, + { + package: "package-b", + versionNumber: "2.0.0.NEXT", + dependencies: [ + { + package: "package-a", + versionNumber: "1.0.0.LATEST" + } + ] + }, + { + package: "package-c", + versionNumber: "1.0.0.NEXT", + dependencies: [ + { + package: "package-b", + versionNumber: "2.0.0.LATEST" + }, + { + package: "package-a", + versionNumber: "1.1.0.LATEST" + } + ] + } + ] + }; + + const transitiveDependencyResolver = new TransitiveDependencyResolver(complexProjectConfig); + const resolvedDependencies = await transitiveDependencyResolver.resolveTransitiveDependencies(); + + // Get dependencies for package-c + const packageCDeps = resolvedDependencies.get('package-c'); + + // Verify package-a appears only once and with the higher version + const packageADeps = packageCDeps?.filter(dep => dep.package === 'package-a'); + expect(packageADeps?.length).toBe(1); + expect(packageADeps?.[0].versionNumber).toBe('1.1.0.LATEST'); + + // Verify package-b is included + const packageBDep = packageCDeps?.find(dep => dep.package === 'package-b'); + expect(packageBDep).toBeTruthy(); + expect(packageBDep?.versionNumber).toBe('2.0.0.LATEST'); + + // Verify the order: package-a should come before package-b due to dependency chain + const packageAIndex = packageCDeps?.findIndex(dep => dep.package === 'package-a'); + const packageBIndex = packageCDeps?.findIndex(dep => dep.package === 'package-b'); + expect(packageAIndex).toBeLessThan(packageBIndex!); + }); + + it("should handle build number versions correctly", async () => { + const buildNumberConfig = { + packageDirectories: [ + { + package: "package-a", + versionNumber: "1.0.0.5", + dependencies: [] + }, + { + package: "package-b", + versionNumber: "1.0.0.NEXT", + dependencies: [ + { + package: "package-a", + versionNumber: "1.0.0.3" + } + ] + } + ] + }; + + const resolver = new TransitiveDependencyResolver(buildNumberConfig); + const resolvedDeps = await resolver.resolveTransitiveDependencies(); + const packageBDeps = resolvedDeps.get('package-b'); + + // Should use the higher build number + const packageADep = packageBDeps?.find(dep => dep.package === 'package-a'); + expect(packageADep?.versionNumber).toBe('1.0.0.3'); + }); + + it("should handle missing version numbers gracefully", async () => { + const missingVersionConfig = { + packageDirectories: [ + { + package: "package-a", + versionNumber: "1.0.0.NEXT", + dependencies: [] + }, + { + package: "package-b", + versionNumber: "1.0.0.NEXT", + dependencies: [ + { + package: "package-a" + // No version number specified + } + ] + } + ] + }; + + const resolver = new TransitiveDependencyResolver(missingVersionConfig); + const resolvedDeps = await resolver.resolveTransitiveDependencies(); + const packageBDeps = resolvedDeps.get('package-b'); + + // Should not throw and should include the dependency + expect(packageBDeps?.find(dep => dep.package === 'package-a')).toBeTruthy(); + }); + + it("should respect specified dependency versions", async () => { + const buildNumberConfig = { + packageDirectories: [ + { + package: "package-a", + versionNumber: "1.0.0.5", // Package A is on version 5 + dependencies: [] + }, + { + package: "package-b", + versionNumber: "1.0.0.NEXT", + dependencies: [ + { + package: "package-a", + versionNumber: "1.0.0.3" // But package B depends on version 3 + } + ] + } + ] + }; + + const resolver = new TransitiveDependencyResolver(buildNumberConfig); + const resolvedDeps = await resolver.resolveTransitiveDependencies(); + const packageBDeps = resolvedDeps.get('package-b'); + + // Should use the specified dependency version + const packageADep = packageBDeps?.find(dep => dep.package === 'package-a'); + expect(packageADep?.versionNumber).toBe('1.0.0.3'); + }); + + it("should throw error on circular dependencies", async () => { + const circularConfig = { + packageDirectories: [ + { + package: "package-a", + versionNumber: "1.0.0.NEXT", + dependencies: [ + { + package: "package-b", + versionNumber: "1.0.0.LATEST" + } + ] + }, + { + package: "package-b", + versionNumber: "1.0.0.NEXT", + dependencies: [ + { + package: "package-a", + versionNumber: "1.0.0.LATEST" + } + ] + } + ] + }; + + const resolver = new TransitiveDependencyResolver(circularConfig); + + // Should throw error due to circular dependency + await expect(resolver.resolveTransitiveDependencies()).rejects.toThrow(/Circular dependency detected.*package-a -> package-b -> package-a/); + }); + + it("should handle deep transitive dependencies", async () => { + const deepConfig = { + packageDirectories: [ + { + package: "package-a", + versionNumber: "1.0.0.NEXT", + dependencies: [] + }, + { + package: "package-b", + versionNumber: "1.0.0.NEXT", + dependencies: [ + { + package: "package-a", + versionNumber: "1.0.0.LATEST" + } + ] + }, + { + package: "package-c", + versionNumber: "1.0.0.NEXT", + dependencies: [ + { + package: "package-b", + versionNumber: "1.0.0.LATEST" + } + ] + }, + { + package: "package-d", + versionNumber: "1.0.0.NEXT", + dependencies: [ + { + package: "package-c", + versionNumber: "1.0.0.LATEST" + } + ] + } + ] + }; + + const resolver = new TransitiveDependencyResolver(deepConfig); + const resolvedDeps = await resolver.resolveTransitiveDependencies(); + const packageDDeps = resolvedDeps.get('package-d'); + + // Should include all transitive dependencies + expect(packageDDeps?.find(dep => dep.package === 'package-a')).toBeTruthy(); + expect(packageDDeps?.find(dep => dep.package === 'package-b')).toBeTruthy(); + expect(packageDDeps?.find(dep => dep.package === 'package-c')).toBeTruthy(); + + // Should maintain correct order + const aIndex = packageDDeps?.findIndex(dep => dep.package === 'package-a'); + const bIndex = packageDDeps?.findIndex(dep => dep.package === 'package-b'); + const cIndex = packageDDeps?.findIndex(dep => dep.package === 'package-c'); + + expect(aIndex).toBeLessThan(bIndex!); + expect(bIndex).toBeLessThan(cIndex!); + }); + + it("should return detailed dependency information with direct and transitive dependencies", async () => { + const config = { + packageDirectories: [ + { + package: "package-a", + versionNumber: "1.0.0.NEXT", + dependencies: [] + }, + { + package: "package-b", + versionNumber: "2.0.0.NEXT", + dependencies: [ + { + package: "package-a", + versionNumber: "1.0.0.LATEST" + } + ] + }, + { + package: "package-c", + versionNumber: "1.0.0.NEXT", + dependencies: [ + { + package: "package-b", + versionNumber: "2.0.0.LATEST" + } + ] + } + ] + }; + + const resolver = new TransitiveDependencyResolver(config); + const result = await resolver.resolveTransitiveDependenciesWithDetails(); + + // Verify structure + expect(result.resolvedDependencies).toBeDefined(); + expect(result.details).toBeDefined(); + + // Check package-c details + const packageCDetails = result.details.get('package-c'); + expect(packageCDetails).toBeDefined(); + + // Verify package-b is a direct dependency of package-c + expect(packageCDetails?.['package-b']).toEqual({ + version: '2.0.0.LATEST', + isDirect: true, + contributors: ['package-c'] // The package itself is tracked as a contributor + }); + + // Verify package-a is a transitive dependency via package-b + expect(packageCDetails?.['package-a']).toEqual({ + version: '1.0.0.LATEST', + isDirect: false, + contributors: ['package-b'] + }); + }); + + it("should track multiple contributors for shared dependencies", async () => { + const config = { + packageDirectories: [ + { + package: "shared-dep", + versionNumber: "1.0.0.NEXT", + dependencies: [] + }, + { + package: "package-a", + versionNumber: "1.0.0.NEXT", + dependencies: [ + { + package: "shared-dep", + versionNumber: "1.0.0.LATEST" + } + ] + }, + { + package: "package-b", + versionNumber: "1.0.0.NEXT", + dependencies: [ + { + package: "shared-dep", + versionNumber: "1.0.0.LATEST" + } + ] + }, + { + package: "root-package", + versionNumber: "1.0.0.NEXT", + dependencies: [ + { + package: "package-a", + versionNumber: "1.0.0.LATEST" + }, + { + package: "package-b", + versionNumber: "1.0.0.LATEST" + } + ] + } + ] + }; + + const resolver = new TransitiveDependencyResolver(config); + const result = await resolver.resolveTransitiveDependenciesWithDetails(); + + const rootDetails = result.details.get('root-package'); + expect(rootDetails).toBeDefined(); + + // Verify shared-dep has both package-a and package-b as contributors + const sharedDepDetails = rootDetails?.['shared-dep']; + expect(sharedDepDetails).toBeDefined(); + expect(sharedDepDetails?.isDirect).toBe(false); + expect(sharedDepDetails?.version).toBe('1.0.0.LATEST'); + expect(sharedDepDetails?.contributors).toContain('package-a'); + expect(sharedDepDetails?.contributors).toContain('package-b'); + expect(sharedDepDetails?.contributors.length).toBe(2); + }); + function verifyUniquePkgs(arr) { let pkgs = {}; for (let i = 0; i < arr.length; i++) { @@ -235,4 +591,3 @@ const projectConfig = { } } }; -