LimitedCollection's maxSize also counts items that pass keepOverLimit

[ImRodry]

Which package is this bug report for?

discord.js

Issue description

1. Create a LimitedCollection with maxSize of 2 and a keepOverLimit function
2. Add 2 elements that pass keepOverLimit
3. Add one that doesn't
4. Check to see that all 3 elements were correctly inserted
5. Add a fourth element that doesn't pass keepOverLimit
6. Check to see that the first element you added that didn't pass keepOverLimit has now been removed in order for your new element to be inserted, leaving you with 3 elements instead of 4.

The way I interpret this keepOverLimit function is that "any element added that passes this condition isn't counted towards the total size limit". I understand there may be other interpretations to this and that the current implementation may be right in the eyes of who interpreted this criteria, but I think this is also a valid way of seeing it and I would like to know if it is possible to change the current implementation to support this.

Code sample

No response

Versions

• discord.js v14.14.1
• Node.js v20.10.0

Issue priority

Low (slightly annoying)

Which partials do you have configured?

Not applicable

Which gateway intents are you subscribing to?

Not applicable

I have tested this issue on a development release

No response

[Qjuh]

A function, which is passed the value and key of an entry, ran to decide to keep an entry past the maximum size

That’s what keepOverLimit is documented to be. And that’s what it is. It decides whether to keep an element although it‘s past the maxSize. Nowhere does it say that it doesn’t count towards the maxSize nor was it ever intended to be that way. So I doubt anyone would implement a breaking change into how this property acts without any reason to do so other than you misunderstanding what it‘s supposed to do.

[ImRodry]

It's exactly because it doesn't say what happens to maxSize that I think it's reasonable to assume it doesn't count, just like it would be reasonable to assume that it does. If you're keeping an element over the limit, why would it count towards that limit and take up the space that could be taken up by other values that don't pass the requirements of that function?

[p4bes]

Hi guys! In my understanding it is ok like @Qjuh described the handling of the count / maxSize.

But I found another strange behaviour here:
The docs say "A function called to check if an entry should be kept when the Collection is at max size."

To me it's strange that the first elements of the collection get cut off in favour of the latest ones, even when they are over limit and not to be kept over limit.

Example:

1. set maxSize to 2
2. add one element which doesn't pass keepOverLimit
3. add one element which pass keepOverLimit
4. Add one element which doesn't pass keepOverLimit

-> Check that the first element is removed and the third one will be inserted.

Question: Is that the intended behavior?
When i read the docs I understand that all elements which are over the limit will be evaluated and possibly not inserted but not remove ones which where previously inserted.

In the upper scenario I would expect the third item not to be inserted as it over the limit and does not pass the keepOverLimit function.
Alternatively the description in the docs could be enhanced.

[sohooc]

While investigating this issue we discovered some peculiar behaviour of this class.
The update behavior of elements inside a LimitedCollection differs depending on the given maxSize.
If the maxSize is not equal to 0 e.g. 1 it's always possible to update the value of a existing key regardless of the keepOverLimit condition.
While if the maxSize is equal to 0 its only possible to update the value of a given key if the keepOverLimit condition is met.
This has been considered by the PR: #10120

[Qjuh]

Hi guys! In my understanding it is ok like @Qjuh described the handling of the count / maxSize.

But I found another strange behaviour here: The docs say "A function called to check if an entry should be kept when the Collection is at max size."

To me it's strange that the first elements of the collection get cut off in favour of the latest ones, even when they are over limit and not to be kept over limit.

Example:

1. set maxSize to 2
2. add one element which doesn't pass keepOverLimit
3. add one element which pass keepOverLimit
4. Add one element which doesn't pass keepOverLimit

-> Check that the first element is removed and the third one will be inserted.

Question: Is that the intended behavior? When i read the docs I understand that all elements which are over the limit will be evaluated and possibly not inserted but not remove ones which where previously inserted.

In the upper scenario I would expect the third item not to be inserted as it over the limit and does not pass the keepOverLimit function. Alternatively the description in the docs could be enhanced.

That absolutely is intended behavior. A LimitedCollection acts as a FIFO Queue that’s also a Map. So the oldest elements get deleted to make room for new ones.

and regarding the other issue I explained in the PR why that‘s not an issue at all.