diff --git a/packages/compat/src/build-compat-addon.ts b/packages/compat/src/build-compat-addon.ts index df9f2caa0..451bc9b2d 100644 --- a/packages/compat/src/build-compat-addon.ts +++ b/packages/compat/src/build-compat-addon.ts @@ -11,7 +11,7 @@ import EmptyPackageTree from './empty-package-tree'; export default function cachedBuildCompatAddon(originalPackage: Package, v1Cache: V1InstanceCache): Node { let tree = buildCompatAddon(originalPackage, v1Cache); if (!originalPackage.mayRebuild) { - tree = OneShot.create(tree, originalPackage.name); + tree = new OneShot(tree, originalPackage.name); } return tree; } diff --git a/packages/compat/src/one-shot.ts b/packages/compat/src/one-shot.ts index 4a993bc19..11564c682 100644 --- a/packages/compat/src/one-shot.ts +++ b/packages/compat/src/one-shot.ts @@ -17,21 +17,10 @@ class NerfHeimdallBuilder extends Builder { buildHeimdallTree() {} } -let seen = new WeakMap(); - // Wraps a broccoli tree such that it (and everything it depends on) will only // build a single time. export default class OneShot extends Plugin { - static create(originalTree: Node, privateAddonName: string) { - let output = seen.get(originalTree); - if (!output) { - output = new this(originalTree, privateAddonName); - seen.set(originalTree, output); - } - return output; - } - - private constructor(private inner: Node | null, private addonName: string) { + constructor(private inner: Node | null, private addonName: string) { // from broccoli's perspective, we don't depend on any input trees! super([], { annotation: `@embroider/compat: ${addonName}`, diff --git a/packages/compat/src/v1-addon.ts b/packages/compat/src/v1-addon.ts index 6e48d0011..410348c42 100644 --- a/packages/compat/src/v1-addon.ts +++ b/packages/compat/src/v1-addon.ts @@ -14,7 +14,6 @@ import { mergeWithAppend } from './merges'; import { AddonMeta, NodeTemplateCompiler, - debug, PackageCache, Resolver, extensionsPattern, @@ -135,7 +134,7 @@ export default class V1Addon { } // Optional extensible hook for pruning down the list of redundant addon - // instances produces by the classic ember-cli architecture. ember-cli + // instances produced by the classic ember-cli architecture. ember-cli // instantiates each addon *per consumer*, not per package. So a given package // will have many addon instances, and Embroider dutifully produces a V1Addon // instance for each one, and then needs to mimic the classic smooshing @@ -150,8 +149,24 @@ export default class V1Addon { // the highest precedence, meaning its files would win under classic // smooshing. reduceInstances(instances: V1Addon[]): V1Addon[] { - // the default beahvior is that all copies matter - return instances; + // Our default implementation will deduplicate instances that have the same + // cacheKeyForTree for *all* the known tree names. + let seenKeys = new Set(); + let keptInstances: V1Addon[] = []; + for (let instance of instances) { + let key = cacheKeyForAddon(instance.addonInstance); + if (!key) { + // entirely uncacheable + keptInstances.push(instance); + } else if (!seenKeys.has(key)) { + // cacheable and new + keptInstances.push(instance); + seenKeys.add(key); + } else { + // dropped as redundant + } + } + return keptInstances; } // this is only defined when there are custom AST transforms that need it @@ -454,14 +469,12 @@ export default class V1Addon { } protected stockTree(treeName: AddonTreePath): Node { - return this.throughTreeCache(treeName, 'stock', () => { - // adjust from the legacy "root" to our real root, because our rootTree - // uses our real root but the stock trees are defined in terms of the - // legacy root - let srcDir = relative(this.root, join(this.addonInstance.root, this.addonInstance.treePaths[treeName])); - let opts = Object.assign({ srcDir }, this.stockTreeFunnelOptions(treeName)); - return buildFunnel(this.rootTree, opts); - })!; + // adjust from the legacy "root" to our real root, because our rootTree + // uses our real root but the stock trees are defined in terms of the + // legacy root + let srcDir = relative(this.root, join(this.addonInstance.root, this.addonInstance.treePaths[treeName])); + let opts = Object.assign({ srcDir }, this.stockTreeFunnelOptions(treeName)); + return buildFunnel(this.rootTree, opts); } @Memoize() @@ -547,25 +560,7 @@ export default class V1Addon { } get v2Tree(): Node { - return this.throughTreeCache( - // these are all the kinds of trees that ember-cli's tree cache - // understands. We need them all here because if *any* of these are - // uncacheable, we want our whole v2 tree to be treated as uncacheable. - [ - 'app', - 'addon', - 'addon-styles', - 'addon-templates', - 'addon-test-support', - 'public', - 'styles', - 'templates', - 'test-support', - 'vendor', - ], - 'v2Tree', - () => mergeTrees(this.v2Trees, { overwrite: true }) - ); + return mergeTrees(this.v2Trees, { overwrite: true }); } // this is split out so that compatibility shims can override it to add more @@ -589,44 +584,6 @@ export default class V1Addon { return trees; } - protected throughTreeCache(nameOrNames: string | string[], category: string, fn: () => Node): Node; - protected throughTreeCache( - nameOrNames: string | string[], - category: string, - fn: () => Node | undefined - ): Node | undefined { - let cacheKey: string | undefined; - if (typeof this.addonInstance.cacheKeyForTree === 'function') { - let names = Array.isArray(nameOrNames) ? nameOrNames : [nameOrNames]; - cacheKey = names.reduce((accum: string | undefined, name) => { - if (accum == null) { - // a previous name was uncacheable, so we're entirely uncacheable - return undefined; - } - let key = this.addonInstance.cacheKeyForTree?.(name); - if (key) { - return accum + key; - } else { - return undefined; - } - }, ''); - if (cacheKey) { - cacheKey = cacheKey + category; - let cachedTree = this.app.addonTreeCache.get(cacheKey); - if (cachedTree) { - debug('cache hit %s %s %s', this.name, nameOrNames, category); - return cachedTree; - } - } - } - debug('cache miss %s %s %s', this.name, nameOrNames, category); - let tree = fn(); - if (tree && cacheKey) { - this.app.addonTreeCache.set(cacheKey, tree); - } - return tree; - } - // In general, we can't reliably run addons' custom `treeFor()` methods, // because they recurse in a way that we absolutely don't want. // @@ -667,28 +624,25 @@ export default class V1Addon { name: string, { neuterPreprocessors } = { neuterPreprocessors: false } ): Node | undefined { - // @ts-expect-error have no idea why throughTreeCache overload is not working here.. - return this.throughTreeCache(name, 'original', () => { - // get the real addon as we're going to patch and restore `preprocessJs` - const realAddon = getRealAddon(this.addonInstance); - let original; - try { - if (neuterPreprocessors) { - original = realAddon.preprocessJs; - realAddon.preprocessJs = function (tree: Node) { - return tree; - }; - } - if (this.suppressesTree(name)) { - return undefined; - } - return this.addonInstance._treeFor(name); - } finally { - if (neuterPreprocessors) { - realAddon.preprocessJs = original; - } + // get the real addon as we're going to patch and restore `preprocessJs` + const realAddon = getRealAddon(this.addonInstance); + let original; + try { + if (neuterPreprocessors) { + original = realAddon.preprocessJs; + realAddon.preprocessJs = function (tree: Node) { + return tree; + }; } - }); + if (this.suppressesTree(name)) { + return undefined; + } + return this.addonInstance._treeFor(name); + } finally { + if (neuterPreprocessors) { + realAddon.preprocessJs = original; + } + } } protected treeForAddon(built: IntermediateBuild): Node | undefined { @@ -1132,3 +1086,38 @@ const stubbedSuper = () => { stubbedSuper.treeFor = () => { return markedEmptyTree; }; + +function cacheKeyForAddon(instance: AddonInstance): string | undefined { + // these are all the kinds of trees that ember-cli's tree cache understands. + // We deduplicate at the level of whole instances, not individual trees, so + // our cache key in the combination of all of these. + let names = [ + 'app', + 'addon', + 'addon-styles', + 'addon-templates', + 'addon-test-support', + 'public', + 'styles', + 'templates', + 'test-support', + 'vendor', + ]; + + let cacheKey: string | undefined; + if (typeof instance.cacheKeyForTree === 'function') { + cacheKey = names.reduce((accum: string | undefined, name) => { + if (accum == null) { + // a previous name was uncacheable, so we're entirely uncacheable + return undefined; + } + let key = instance.cacheKeyForTree?.(name); + if (key) { + return accum + key; + } else { + return undefined; + } + }, ''); + } + return cacheKey; +} diff --git a/packages/compat/src/v1-app.ts b/packages/compat/src/v1-app.ts index 5fe07ed87..874cbdbd9 100644 --- a/packages/compat/src/v1-app.ts +++ b/packages/compat/src/v1-app.ts @@ -118,11 +118,6 @@ export default class V1App { return this.requireFromEmberCLI('./lib/utilities/ember-app-utils'); } - @Memoize() - get addonTreeCache(): Map { - return new Map(); - } - @Memoize() get preprocessRegistry() { return this.requireFromEmberCLI('ember-cli-preprocess-registry/preprocessors');