Able to modify ReadonlyCollection

[TmGL]

Which package is this bug report for?

collection

Issue description

I doubt this matters too much, but here it is anyway.

Steps to reproduce:

1. Call any method which has the this return type upon the Collection.
2. The method will return a Collection which is no longer of the ReadonlyCollection type...
3. ...thus, you can call methods such as .set() and .delete() on the Collection.

Code sample

const testColl: ReadonlyCollection<string, number> = new Collection<string, number>([
    ['id', 123]
]);

testColl.tap(() => {}).set('unexpected', 0);
testColl.each(() => {}).delete('id');

console.log(testColl); // Collection(1) [Map] { 'unexpected' => 0 }

Versions

• @discordjs/collection 2.1.1
• discord.js 14.16.2
• typescript 5.3.3
• NodeJS 20.14.0
• Windows 22H2

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

[Waheedsys]

can i work on this?

[Renegade334]

This comes with other questions. For example, a lot of methods have callback parameters with this types, which are also resolved to Collection by the compiler; technically, these also give the opportunity to resolve a ReadonlyCollection to a Collection and call mutating methods on it.

Ultimately, the options are:

1. Refactor ReadonlyCollection to duplicate the definitions for all non-mutating Collection methods, rather than grabbing them from the Collection interface (example).
   This is the approach taken by Microsoft for Readonly* interfaces within the TypeScript library, and would provide a self-contained interface where TypeScript's this type refers to ReadonlyCollection, which would render the method signatures fully non-mutable (including callback parameters etc.). However, this would be an awful lot of duplication here, and would impact on maintainability.
2. Leave things as they are, on the understanding that ReadonlyCollection is nothing more than Collection without the mutating methods, and isn't going to stop you from deriving a mutable Collection from the remaining methods.

[didinele]

We spoke about this internally and I think we wanted to do something along the lines of 1, but dynamic. You do raise a good point with the callbacks though. I'll try to draft up a PR.

My main concern is that I'm probably obliterating the docs readability for ReadonlyCollection, but.. I think it's probably already pretty bad & also not that important.