fix(resolve): separate package cache per environment #18319
Conversation
|
|
| @@ -64,6 +65,7 @@ export class PartialEnvironment { | |||
| this.name = name | |||
| this._topLevelConfig = topLevelConfig | |||
| this._options = options | |||
| const packageCache: PackageCache = new Map() | |||
There was a problem hiding this comment.
I think this is going to far. The PackageCache has two mixed caches:
a. The path -> PackageData cache
b. the resolved cache in each PackageData (the getResolvedCache/setResolvedCache pair)
We still need (a) to be per-config and not per-environment. This cache doesn't depend on environment-specific options. We need to make only (b) per-environment.
What I was thinking is that PackageCache will remain as-is, the cache from (b) is removed from PackageData. And a new ResolvedCache is created per-environment that will expose resolvedCache.get(packageData, key). This cache is passed to the resolve plugin that is the only place where it is used.
There was a problem hiding this comment.
IIUC making (b) per-environment is not enough. It also needs to be separated whether it's a resolve for require or import. Otherwise, #16631 would be still broken.
There was a problem hiding this comment.
Maybe I'm missing something. I would expect the PackageData (without the Resolved Cache part) would be the same for both in that case 🤔
There was a problem hiding this comment.
Good point. Splitting entire PackageData seems redundant. Let me iterate a bit more.
Should we go with #18302 for now? I can follow up separately with your idea.
There was a problem hiding this comment.
IIUC making (b) per-environment is not enough. It also needs to be separated whether it's a resolve for
requireorimport. Otherwise, #16631 would be still broken.
For this, I was thinking we still need to expand isRequire as a cache key, which we aren't doing. But, for other flags conditions, mainFields, ..., we can assume those are static per environment, so we don't need to include it as cache key, which was the heavy part of #18302
Description
Alternative approach #18302 (comment)
This PR separates
PackageCacheper environment usingenvironment.configproxy. I replacedconfig.packageCachewithenvironment.config.packageCachethrough the code base (seems straight-forward exceptwatchPackageDataPlugin).