Webpack overwrites custom objects passed into resolve.unsafeCache

[jpbriggs408]

Bug report

When upgrading from Webpack 4 to Webpack 5, we identified an issue where custom Proxy cache objects passed into resolve.unsafeCache are overwritten by the removeOperations function. This function is executed recursively on resolve options, transforming the Proxy cache into a plain, empty object right before the options are utilized by the enhanced-resolve library.

In enhanced-resolve, we should be able to pass a boolean or an object to unsafeCache. Passing an object allows it to serve as the cache directly. This functionality is crucial for maintaining custom caching logic in complex projects.

What is the current behavior?
The custom Proxy cache object is replaced with a plain, empty object due to the recursive execution of removeOperations.

Steps to Reproduce

1. Implement a custom Proxy object for caching in Webpack 5's resolve options. For example, this configuration invalidates the cache if the file does not exist on disk:

const fileSystemAwareCache = new Proxy({},{
    get(target, key, receiver) {
        const entry = Reflect.get(target, key, receiver);
        if (entry && fs.existsSync(entry.path)) {
            return entry;
        }

        Reflect.deleteProperty(target, key);
        return undefined;
    },
  
    set(target, key, value, receiver) {
        return Reflect.set(target, key, value, receiver);
      }
});

// and in our config:
resolve: {
    unsafeCache: fileSystemAwareCache,
}

2. Observe that upon startup, the custom Proxy is utilized, but once a file is renamed and a server is running, the cache entry is sourced from an unexpected location, bypassing the custom Proxy.

I've created a minimal reproducible example here: https://github.com/jpbriggs408/Webpack-UnsafeCache-ModuleBuildError-Repro.

What is the expected behavior?

The custom Proxy cache object should be consistently used for caching across all operations, maintaining its integrity and functionality throughout the Webpack resolution process.

Possible Solutions

• Consider revising the handling of objects passed to resolve.unsafeCache to preserve custom caching mechanisms.
• Rather than preventing the issue at the source, another approach is to allow developers to intervene or correct the cache's state dynamically, exposing a way to recover when using resolve.unsafeCache
• Amending the cache key used in enhanced-resolve is a potential solution, though this has not been explored in depth.
• Investigating the use of module.unsafeCache with a function to re-resolve requests dynamically for non-existent modules.

Notes
Enabling filesystem caching and disabling resolve.unsafeCache re-gains some lost perf benefits, though ideally, we should be able to have both. In our (extremely large) monorepo, the overhead of the filesystem check is minimal and having both options is a clear perf benefit. This actually makes Webpack 5 with filesystem caching only marginally slower for us in dev than our Webpack 4 version with the above simple filesystem check enabled for the resolver cache. Also, special thanks to @joebeachjoebeach for helping connect the final dots with this investigation!

Other relevant information:
webpack version: 5.89.0
Node.js version: 16.13.2
Operating System: CentOS 7
Additional tools: N/A

Previous Context
I previously opened a GitHub Discussion when I began this investigation.

TL;DR Summary
We've discovered an issue in Webpack 5 where custom Proxy cache objects specified in resolve.unsafeCache are overwritten by the removeOperations function in cleverMerge.js. This function's recursive operation on resolve options transforms our Proxy cache into a plain, empty object, just before being passed to enhanced-resolve. Despite enhanced-resolve's capability to use an object directly as the cache, this behavior negates the utility of custom Proxy caches, affecting developers working on complex projects.

We're seeking insights on preserving custom caching logic within Webpack's ecosystem. Feedback and suggestions would be greatly appreciated to navigate and “resolve” this challenge.

Thanks in advance!

[alexander-akait]

I see your problem

[alexander-akait]

I found two solution:

1. in convertToResolveOptions:

const unsafeCache = options.unsafeCache;
const normalizedOptions = removeOperations(
  resolveByProperty(options, "byDependency", dependencyType)
);
if (unsafeCache !== undefined) {
  normalizedOptions.unsafeCache = unsafeCache;
}

return normalizedOptions;

This is not a universal solution, we assume that only there a proxy can be used for unsafeCache

2. Implement const removeOperations = (obj, ignoredKeys = []) => { /* ... */ } and so put unsafeCache in ignoredKeys, so keep them as is without deep merging, like the 1. but just improving

3. Make Proxy to be VALUE_TYPE_ATOM in getValueType, but it is only for Node.js, so if somebody will want to bundle webpack for browser, it will not work

const getValueType = value => {
	if (value === undefined) {
		return VALUE_TYPE_UNDEFINED;
	} else if (value === DELETE) {
		return VALUE_TYPE_DELETE;
	} else if (util.types.isProxy(value)) {
		return VALUE_TYPE_ATOM;
	} else if (Array.isArray(value)) {
		if (value.lastIndexOf("...") !== -1) return VALUE_TYPE_ARRAY_EXTEND;
		return VALUE_TYPE_ATOM;
	} else if (
		typeof value === "object" &&
		value !== null &&
		(!value.constructor || value.constructor === Object)
	) {
		return VALUE_TYPE_OBJECT;
	}
	return VALUE_TYPE_ATOM;
};

All solutions works with your repo. But yeah, that is intresting problem, because we use getValueType in many places, and so these places will not work with Proxy

Want to get some feedback from you

[alexander-akait]

Another solution like 3. but we can make it smarty:

if (util.types.isProxy(value) || value.__isProxy) {
}

and

fileSystemAwareCache.__isProxy = true;

so it give ability to mark Proxy in a browser

[jpbriggs408]

Thank you for the quick response!

Options 1 and 2 seem like the safest immediate fixes with minimal impact, though we acknowledge they might not cover all future scenarios.

Option 3 (and its browser-compatible variant) do offer a more universal fix. It does seem like the most ideal solution to our specific problem. However, our main concern revolves around potential side-effects across other getValueType usages. It would be great to know more about the specific operations removeOperations aims to exclude from your perspective. That would help us to gauge this option's viability better. That being said, we only pass in our custom cache object in the development environment, but browser compatibility is important to us.

[icyflame0]

see if this helps

const fileSystemAwareCache = new Proxy({}, {
get(target, key, receiver) {
if (key in target) {
const entry = target[key];
if (entry && fs.existsSync(entry.path)) {
return entry;
} else {
delete target[key];
}
}
return undefined;
},

set(target, key, value, receiver) {
    target[key] = value;
    return true;
}

});

[alexander-akait]

@jpbriggs408 There is a fix #18152, I decide to avoid proxy check, because it is not universal (we can't undestand when it is a plain object or a proxied object, except node.js utils ), so let's just keep unsafeCache as is and avoid the object creation

[jpbriggs408]

@alexander-akait thank you so much! I think this solution makes a lot of sense.

If documentation needs to be updated, I'm happy to take a crack at it. Let me know, and thanks again!

[alexander-akait]

I don't thank we need document it, because it is like a bug

[jpbriggs408]

Totally reasonable! In webpack's documentation, it isn't clear that an object can be passed, but that is outlined in enhanced-resolve's code which is sufficient enough to me.

[mpmartinez-etsy]

@alexander-akait thank you for this fix, we really appreciate it! Would we be able to get a release cut with this fix in it at your convenience?

[alexander-akait]

Yeah, this week