fix(deps): exclude @types/* packages from component peer dependencies #10186
+192
−95
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -570,107 +570,158 @@ export class ApplyOverrides { | |
| } | ||
|
|
||
| const originallyExists: string[] = []; | ||
| const missingPackages = this.getMissingPackagesFromIssues(); | ||
|
|
||
| ['dependencies', 'devDependencies', 'peerDependencies'].forEach((field) => { | ||
| forEach(autoDetectOverrides[field], (pkgVal, pkgName) => { | ||
| this.applyAutoDetectedOverrideForPackage(field, pkgName, pkgVal, originallyExists, missingPackages); | ||
| }); | ||
| }); | ||
| } | ||
|
|
||
| private getMissingPackagesFromIssues(): string[] { | ||
| const missingData = this.issues.getIssueByName('MissingPackagesDependenciesOnFs')?.data as | ||
| | MissingPackagesData[] | ||
| | undefined; | ||
| if (!missingData) return []; | ||
| return uniq(missingData.map((d) => d.missingPackages).flat()); | ||
| } | ||
|
|
||
| private isPackageAutoDetected(pkgName: string, originallyExists: string[], missingPackages: string[]): boolean { | ||
| // Check component dependencies | ||
| const existsInAnyCompDeps = | ||
| this.allDependencies.dependencies.some((dep) => dep.packageName === pkgName) || | ||
| this.allDependencies.devDependencies.some((dep) => dep.packageName === pkgName) || | ||
| this.allDependencies.peerDependencies.some((dep) => dep.packageName === pkgName); | ||
|
|
||
| // Check originAllPackagesDependencies instead of allPackagesDependencies | ||
| // as it might be already removed at this point if it was set with "-" in runtime/dev | ||
| const existsInOriginalPkgDeps = | ||
| this.originAllPackagesDependencies.packageDependencies[pkgName] || | ||
| this.originAllPackagesDependencies.devPackageDependencies[pkgName] || | ||
| this.originAllPackagesDependencies.peerPackageDependencies[pkgName]; | ||
|
|
||
| // Check if it was originally exists in the component | ||
| // This handles policies like: dependencies: {"my-dep": "-"}, devDependencies: {"my-dep": "1.0.0"} | ||
| // where we might remove it before getting to devDeps | ||
| return ( | ||
| existsInAnyCompDeps || | ||
| !!existsInOriginalPkgDeps || | ||
| originallyExists.includes(pkgName) || | ||
| missingPackages.includes(pkgName) | ||
| ); | ||
| } | ||
|
|
||
| private applyAutoDetectedOverrideForPackage( | ||
| field: string, | ||
| pkgName: string, | ||
| pkgVal: string, | ||
| originallyExists: string[], | ||
| missingPackages: string[] | ||
| ): void { | ||
| if (this.overridesDependencies.shouldIgnorePeerPackage(pkgName)) { | ||
| return; | ||
| } | ||
|
|
||
| if (!this.isPackageAutoDetected(pkgName, originallyExists, missingPackages)) { | ||
| return; | ||
| } | ||
|
|
||
| originallyExists.push(pkgName); | ||
|
|
||
| const existsInCompsDeps = this.allDependencies.dependencies.find((dep) => dep.packageName === pkgName); | ||
| const existsInCompsDevDeps = this.allDependencies.devDependencies.find((dep) => dep.packageName === pkgName); | ||
| const existsInCompsPeerDeps = this.allDependencies.peerDependencies.find((dep) => dep.packageName === pkgName); | ||
|
|
||
| // When changing peer dependency we want it to be stronger than the other types | ||
| if (field === 'peerDependencies') { | ||
| const shouldSkip = this.handlePeerDependencyOverride(pkgName, existsInCompsDeps, existsInCompsDevDeps); | ||
| if (shouldSkip) return; | ||
| } | ||
|
|
||
| // Clear package from all dependency types to avoid duplicates | ||
| // This fixes cases where a package moves between dep types in env.jsonc | ||
| this.removePackageFromAllDependencyTypes(pkgName); | ||
|
|
||
| this.applyPackageVersion(field, pkgName, pkgVal, existsInCompsDeps, existsInCompsDevDeps, existsInCompsPeerDeps); | ||
| } | ||
|
|
||
| /** | ||
| * Handle peer dependency specific logic. | ||
| * @returns true if the package should be skipped (not added to dependencies) | ||
| */ | ||
| private handlePeerDependencyOverride( | ||
| pkgName: string, | ||
| existsInCompsDeps: Dependency | undefined, | ||
| existsInCompsDevDeps: Dependency | undefined | ||
| ): boolean { | ||
| delete this.allPackagesDependencies.devPackageDependencies[pkgName]; | ||
| delete this.allPackagesDependencies.packageDependencies[pkgName]; | ||
|
|
||
| // @types/* packages should not be added to components at all when in env peers. | ||
| // When a component is installed in a workspace, its env is installed as well. | ||
| // Packages listed as peers in env.jsonc become runtime dependencies of the env itself, | ||
| // so they're always installed alongside the env. Since @types/* packages are only needed | ||
| // for TypeScript compilation (which the env handles), there's no need for components | ||
| // to have them in their own dependencies. | ||
| if (pkgName.startsWith('@types/')) { | ||
| delete this.allPackagesDependencies.peerPackageDependencies[pkgName]; | ||
| return true; | ||
| } | ||
|
Comment on lines
+661
to
+670
There was a problem hiding this comment. 1. Stale missing @types issue handlePeerDependencyOverride() removes @types/* packages from the component dependency maps and returns early, but does not remove the same package from an already-populated MissingPackagesDependenciesOnFs issue. This can incorrectly keep the component in a tag-blocking “missing packages” state even though @types/* was intentionally excluded from the component dependencies. Agent Prompt
|
||
|
|
||
| if (existsInCompsDeps) { | ||
| this.allDependencies.dependencies = this.allDependencies.dependencies.filter( | ||
| (dep) => dep.packageName !== pkgName | ||
| ); | ||
| } | ||
| if (existsInCompsDevDeps) { | ||
| this.allDependencies.devDependencies = this.allDependencies.devDependencies.filter( | ||
| (dep) => dep.packageName !== pkgName | ||
| ); | ||
| } | ||
|
|
||
| return false; | ||
| } | ||
|
|
||
| private removePackageFromAllDependencyTypes(pkgName: string): void { | ||
| delete this.allPackagesDependencies.packageDependencies[pkgName]; | ||
| delete this.allPackagesDependencies.devPackageDependencies[pkgName]; | ||
| delete this.allPackagesDependencies.peerPackageDependencies[pkgName]; | ||
| } | ||
|
|
||
| private applyPackageVersion( | ||
| field: string, | ||
| pkgName: string, | ||
| pkgVal: string, | ||
| existsInCompsDeps: Dependency | undefined, | ||
| existsInCompsDevDeps: Dependency | undefined, | ||
| existsInCompsPeerDeps: Dependency | undefined | ||
| ): void { | ||
| // If it exists in comps deps / comp dev deps, we don't want to add it to the allPackagesDependencies | ||
| // as it will make the same dep both a dev and runtime dep | ||
| // since we are here only for auto detected deps, it means we already resolved the version correctly | ||
| // so we don't need to really modify the version | ||
| // also the version here might have a range (^ or ~ for example) so we can't | ||
| // just put it as is, as it is not valid for component deps to have range | ||
| const shouldAddPackage = | ||
| pkgVal !== MANUALLY_REMOVE_DEPENDENCY && | ||
| ((!existsInCompsDeps && !existsInCompsDevDeps) || field === 'peerDependencies'); | ||
|
|
||
| if (!shouldAddPackage) return; | ||
|
|
||
| const key = DepsKeysToAllPackagesDepsKeys[field]; | ||
|
|
||
| if ((existsInCompsDeps || existsInCompsDevDeps) && field === 'peerDependencies') { | ||
| const comp = (existsInCompsDeps ?? existsInCompsDevDeps) as Dependency; | ||
| comp.versionRange = pkgVal; | ||
| this.allDependencies.peerDependencies.push(comp); | ||
| } else { | ||
| this.allPackagesDependencies[key][pkgName] = pkgVal; | ||
| } | ||
|
|
||
| if (existsInCompsPeerDeps) { | ||
| existsInCompsPeerDeps.versionRange = pkgVal; | ||
| } | ||
| } | ||
|
|
||
| private async applyAutoDetectedPeersFromEnvOnEnvItSelf(): Promise<void> { | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.