Bug Tracker

Opened 5 years ago

Closed 5 years ago

#15222 closed bug (worksforme)

cacheLength is ignored in Sizzle's createCache()

Reported by: nessup Owned by:
Priority: high Milestone: None
Component: selector Version: 1.9.1
Keywords: sizzle, cache, cachelimit Cc:
Blocked by: Blocking:

Description

It appears that elements are added to Sizzle caches regardless of whether the cache limit has been exceeded. This bug affects 1.9.1 and appears to also affect 2.1.1. Consider:

function createCache() {
	var cache,
		keys = [];

	return (cache = function( key, value ) {
		// Use (key + " ") to avoid collision with native prototype properties (see Issue #157)
		if ( keys.push( key += " " ) > Expr.cacheLength ) {
			// Only keep the most recent entries
			cache[ keys.shift() ] = null;
		}
		return (cache[ key ] = value);
	});
}

While the keys array will always be within the cache limit, the actual cache instance will store the cached object regardless of whether the limit has been exceeded.

Barring verbosity, I think that this method should be written as follows:

function createCache() {
	var cache,
		keys = [];

	return (cache = function( key, value ) {
		// Use (key + " ") to avoid collision with native prototype properties (see Issue #157)
		if ( keys.push( key += " " ) > Expr.cacheLength ) {
			// Only keep the most recent entries
			delete cache[ keys.shift() ];
			return value;
		}
		else {
			return (cache[ key ] = value);
		}
	});
}

Notice that, now, the cache variable only increases in length when key.length <= Expr.cacheLength. I think this is truer to the intent of cacheLimit. However, I'm not sure whether return value; makes sense in the affirmative branch of the control flow, and need assistance with this particular point.

Am I correct here? Or is cache length only meant to limit the size of the keys array? If so, why?

Change History (1)

comment:1 Changed 5 years ago by gibson042

Resolution: worksforme
Status: newclosed
Note: See TracTickets for help on using tickets.