Skip to content

Conversation

@Renegade334
Copy link
Contributor

@Renegade334 Renegade334 commented Jul 22, 2024

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

Small changes relating to the typed method definitions for reduce() and reduceRight():

  • Fix the "without-initial-value" signature of reduceRight().
    • When initialValue was not provided, this method would previously accept a callback with any arbitrary type for previousValue, or infer the type as unknown without explicit type annotation:
      declare const coll: Collection<string, number>;
      coll.reduceRight((a, x) => a + x); // error: 'a' is of type 'unknown'
      coll.reduceRight((a: string, x) => a.repeat(x)); // compiles OK, but will raise a TypeError at runtime
    • With this change, if an initial value for the accumulator is not provided, the callback's previousValue type is always the collection value type, and will raise an error otherwise.
  • Separate out "accumulator type = collection value type" and "accumulator type = arbitrary type" overload signatures, à la TypeScript's library definitions for Array.prototype.reduce.
    • This changes the type behaviour in one specific set of cases, which is where a value for initialValue is provided with a type that is assignable to, but not equal to, the collection value type:
      declare const coll: Collection<string, number>;
      declare const initial: 1 | 2 | 3;
      
      coll.reduce((a, x) => a + x, initial); // Type 'number' is not assignable to type '1 | 2 | 3'
    • Previously, InitialValue would be inferred as 1 | 2 | 3, leading to compilation errors.
    • With this change, if initialValue is assignable to the collection value type, the method signature will preferentially use the collection value type for the callback, rather than narrowing to the type of initialValue. This matches the behaviour of Array.prototype.reduce in TypeScript.

Also adds one missing reduceRight() test (reduce empty collection with initial value).

@vercel
Copy link

vercel bot commented Jul 22, 2024

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 Jul 28, 2024 1:31pm
discord-js-guide ⬜️ Ignored (Inspect) Visit Preview Jul 28, 2024 1:31pm

@codecov
Copy link

codecov bot commented Jul 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 50.75%. Comparing base (bf6761a) to head (abe7cd0).

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #10405      +/-   ##
==========================================
+ Coverage   50.73%   50.75%   +0.01%     
==========================================
  Files         228      228              
  Lines       20646    20652       +6     
  Branches     1253     1261       +8     
==========================================
+ Hits        10475    10481       +6     
  Misses      10127    10127              
  Partials       44       44              
Flag Coverage Δ
collection 100.00% <100.00%> (ø)
rest 92.68% <ø> (ø)
ws 51.88% <ø> (ø)

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.

@Renegade334 Renegade334 force-pushed the collection-reduce-signatures branch from 34bee55 to 8532df9 Compare July 24, 2024 20:15
@Renegade334 Renegade334 requested a review from kyranet July 24, 2024 20:17
@kodiakhq kodiakhq bot merged commit 6b38335 into discordjs:main Jul 28, 2024
@Renegade334 Renegade334 deleted the collection-reduce-signatures branch July 28, 2024 21:07
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.

4 participants