Skip to content

feat(collection): honour subclassing via @@species in static methods #10723

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

Merged
merged 4 commits into from
Jan 26, 2025

Conversation

Renegade334
Copy link
Contributor

Please describe the changes this PR makes and why it should be merged:

Collection methods feature subclassing support via Symbol.species. This currently doesn't apply to the one static method, which calls the Collection constructor specifically.

This change brings the behaviour of static methods in line with instance methods, such that DerivedCollection.combineEntries(...) returns a DerivedCollection, not a Collection.

Copy link

vercel bot commented Jan 21, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

2 Skipped Deployments
Name Status Preview Comments Updated (UTC)
discord-js ⬜️ Ignored (Inspect) Visit Preview Jan 26, 2025 1:08pm
discord-js-guide ⬜️ Ignored (Inspect) Visit Preview Jan 26, 2025 1:08pm

@Renegade334 Renegade334 force-pushed the collection-static-inheritance branch 2 times, most recently from 8166bd8 to 472b938 Compare January 21, 2025 18:07
@almeidx
Copy link
Member

almeidx commented Jan 21, 2025

Can you add tests for this?

@Renegade334
Copy link
Contributor Author

Renegade334 commented Jan 21, 2025

Subclassing is currently not tested at all, which is why I didn't add any. If tests are desirable here, then they probably also need adding for the other methods as well?

@Renegade334 Renegade334 force-pushed the collection-static-inheritance branch from 472b938 to c2e4656 Compare January 24, 2025 12:51
Copy link

codecov bot commented Jan 24, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 38.60%. Comparing base (052ed7f) to head (621741f).
Report is 16 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10723      +/-   ##
==========================================
+ Coverage   38.00%   38.60%   +0.59%     
==========================================
  Files         239      239              
  Lines       14894    14664     -230     
  Branches     1392     1390       -2     
==========================================
  Hits         5661     5661              
+ Misses       9221     8991     -230     
  Partials       12       12              
Flag Coverage Δ
collection 100.00% <100.00%> (ø)
proxy 64.63% <ø> (ø)
rest 87.33% <ø> (ø)
ws 34.68% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@kodiakhq kodiakhq bot merged commit b7e0fe3 into discordjs:main Jan 26, 2025
24 checks passed
@almeidx almeidx added this to the collection 3.0.0 milestone Jan 26, 2025
@Renegade334 Renegade334 deleted the collection-static-inheritance branch January 26, 2025 13:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

5 participants