Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

LimitedCollection's maxSize also counts items that pass keepOverLimit #10062

Open
ImRodry opened this issue Dec 29, 2023 · 2 comments
Open

LimitedCollection's maxSize also counts items that pass keepOverLimit #10062

ImRodry opened this issue Dec 29, 2023 · 2 comments
Labels
bug need repro packages:discord.js

Comments

@ImRodry
Copy link
Contributor

ImRodry commented Dec 29, 2023

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
@ImRodry ImRodry changed the title LimitedCollection's maxSize also contemplates items that pass keepOverLimit LimitedCollection's maxSize also counts items that pass keepOverLimit Dec 29, 2023
@Qjuh
Copy link
Contributor

Qjuh commented Dec 29, 2023 •
edited by Jiralite

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
Copy link
Contributor Author

ImRodry commented Dec 29, 2023

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug need repro packages:discord.js
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@ImRodry @Qjuh