From e89708fd9df37d0e5f5f4b0fb5a412b0c6299eca Mon Sep 17 00:00:00 2001 From: Charles Pierce Date: Thu, 18 Mar 2021 16:38:19 -0700 Subject: [PATCH 1/2] Exclude peerDependencies from Engines 'app' directory --- packages/compat/src/compat-app.ts | 2 +- packages/compat/src/moved-package-cache.ts | 3 +++ packages/shared-internals/src/package.ts | 15 +++++++++++++-- 3 files changed, 17 insertions(+), 3 deletions(-) diff --git a/packages/compat/src/compat-app.ts b/packages/compat/src/compat-app.ts index a7246cedc..65406a15a 100644 --- a/packages/compat/src/compat-app.ts +++ b/packages/compat/src/compat-app.ts @@ -176,7 +176,7 @@ class CompatAppAdapter implements AppAdapter { @Memoize() activeAddonChildren(pkg: Package = this.appPackage): AddonPackage[] { - let result = pkg.dependencies.filter(this.isActiveAddon) as AddonPackage[]; + let result = pkg.addonDependencies.filter(this.isActiveAddon) as AddonPackage[]; if (pkg === this.appPackage) { let extras = [this.synthVendor, this.synthStyles].filter(this.isActiveAddon) as AddonPackage[]; result = [...result, ...extras]; diff --git a/packages/compat/src/moved-package-cache.ts b/packages/compat/src/moved-package-cache.ts index 30619deeb..b84908f9d 100644 --- a/packages/compat/src/moved-package-cache.ts +++ b/packages/compat/src/moved-package-cache.ts @@ -314,6 +314,9 @@ function packageProxy(pkg: Package, getMovedPackage: (pkg: Package) => Package) if (prop === 'dependencies') { return pkg.dependencies.map(getMovedPackage); } + if (prop === 'addonDependencies') { + return pkg.addonDependencies.map(getMovedPackage); + } if (prop === 'nonResolvableDeps') { if (!pkg.nonResolvableDeps) { return pkg.nonResolvableDeps; diff --git a/packages/shared-internals/src/package.ts b/packages/shared-internals/src/package.ts index 13e907df5..9c2f24fb5 100644 --- a/packages/shared-internals/src/package.ts +++ b/packages/shared-internals/src/package.ts @@ -167,11 +167,22 @@ export default class Package { @Memoize() get dependencies(): Package[] { - let names = flatMap(this.dependencyKeys, key => Object.keys(this.packageJSON[key] || {})); + return this.loadDependencies(this.dependencyKeys); + } + + @Memoize() + get addonDependencies(): Package[] { + // When looking for child addons, we want to ignore 'peerDependencies' of a given package, to + // align with how ember-cli resolves addons + return this.loadDependencies(this.dependencyKeys.filter(key => key !== 'peerDependencies')); + } + + loadDependencies(keys: ('dependencies' | 'devDependencies' | 'peerDependencies')[]): Package[] { + const names = flatMap(keys, key => Object.keys(this.packageJSON[key] || {})); return names .map(name => { if (this.nonResolvableDeps) { - let dep = this.nonResolvableDeps.get(name); + const dep = this.nonResolvableDeps.get(name); if (dep) { return dep; } From a3bdc91432d0ba474aa27dc5e9c6da2e9e880043 Mon Sep 17 00:00:00 2001 From: Edward Faulkner Date: Thu, 22 Apr 2021 14:42:51 -0400 Subject: [PATCH 2/2] move peerDep check into activeAddonChildren I'm suggesting this change on top of https://github.com/embroider-build/embroider/pull/728 because I'd rather keep this behavior local to child addon discovery and not make it be something that's part of the Package's API. This should still provide the same performance benefit, because even though we will now examine the peerDeps before filtering them out, that work is guarded by the PackageCache and won't actually be repeated unnecessarily. The main thing we're trying to avoid is building many copies of each peerDep per consuming addon. --- packages/compat/src/compat-app.ts | 6 +++++- packages/compat/src/moved-package-cache.ts | 3 --- packages/shared-internals/src/package.ts | 15 ++------------- 3 files changed, 7 insertions(+), 17 deletions(-) diff --git a/packages/compat/src/compat-app.ts b/packages/compat/src/compat-app.ts index 65406a15a..fe5db6ddb 100644 --- a/packages/compat/src/compat-app.ts +++ b/packages/compat/src/compat-app.ts @@ -176,7 +176,11 @@ class CompatAppAdapter implements AppAdapter { @Memoize() activeAddonChildren(pkg: Package = this.appPackage): AddonPackage[] { - let result = pkg.addonDependencies.filter(this.isActiveAddon) as AddonPackage[]; + let result = (pkg.dependencies.filter(this.isActiveAddon) as AddonPackage[]).filter( + // When looking for child addons, we want to ignore 'peerDependencies' of a given package, to + // align with how ember-cli resolves addons + addon => !pkg.packageJSON.peerDependencies?.[addon.name] + ); if (pkg === this.appPackage) { let extras = [this.synthVendor, this.synthStyles].filter(this.isActiveAddon) as AddonPackage[]; result = [...result, ...extras]; diff --git a/packages/compat/src/moved-package-cache.ts b/packages/compat/src/moved-package-cache.ts index b84908f9d..30619deeb 100644 --- a/packages/compat/src/moved-package-cache.ts +++ b/packages/compat/src/moved-package-cache.ts @@ -314,9 +314,6 @@ function packageProxy(pkg: Package, getMovedPackage: (pkg: Package) => Package) if (prop === 'dependencies') { return pkg.dependencies.map(getMovedPackage); } - if (prop === 'addonDependencies') { - return pkg.addonDependencies.map(getMovedPackage); - } if (prop === 'nonResolvableDeps') { if (!pkg.nonResolvableDeps) { return pkg.nonResolvableDeps; diff --git a/packages/shared-internals/src/package.ts b/packages/shared-internals/src/package.ts index 9c2f24fb5..13e907df5 100644 --- a/packages/shared-internals/src/package.ts +++ b/packages/shared-internals/src/package.ts @@ -167,22 +167,11 @@ export default class Package { @Memoize() get dependencies(): Package[] { - return this.loadDependencies(this.dependencyKeys); - } - - @Memoize() - get addonDependencies(): Package[] { - // When looking for child addons, we want to ignore 'peerDependencies' of a given package, to - // align with how ember-cli resolves addons - return this.loadDependencies(this.dependencyKeys.filter(key => key !== 'peerDependencies')); - } - - loadDependencies(keys: ('dependencies' | 'devDependencies' | 'peerDependencies')[]): Package[] { - const names = flatMap(keys, key => Object.keys(this.packageJSON[key] || {})); + let names = flatMap(this.dependencyKeys, key => Object.keys(this.packageJSON[key] || {})); return names .map(name => { if (this.nonResolvableDeps) { - const dep = this.nonResolvableDeps.get(name); + let dep = this.nonResolvableDeps.get(name); if (dep) { return dep; }