[@types/underscore] Collection and Array Tests - SortBy, IndexBy, CountBy, and Invoke by reubenrybnik · Pull Request #46352 · DefinitelyTyped/DefinitelyTyped

reubenrybnik · 2020-07-26T06:48:09Z

Use a meaningful title for the pull request. Include the name of the package modified.
Test the change in your own code. (Compile and run.)
Add or edit tests to reflect the change. (Run with npm test.)
Follow the advice from the readme.
Avoid common mistakes.
Run npm run lint package-name (or tsc if no tslint.json is present).
Provide a URL to documentation or source code which provides context for the suggested changes: https://underscorejs.org/
Include tests for your changes

This PR continues the planned set that will together add up to #45201 and includes the following changes:

Adds tests for sortBy, indexBy, countBy, and invoke.
Updates sortBy, indexBy, countBy, and invoke to use Collection and Iteratee to partially fix Underscore collections functions should take objects #20623.
Updates invoke return types from any and T to any[].
Updates the return type of _Chain.sortBy, _Chain.indexBy, _Chain.countBy, and _Chain.invoke to use the correct wrapped value type V to partially fix @types/underscore error TS2322 for _.chain after upgrade to v1.9 #36308.
Updates the collection type result for _Chain.countBy to be number instead of T.
Updates overloads of groupBy to be more consistent with these overloads by making tweaks to summary comments, making iteratee optional, and adding tests for all iteratee types.
Updates test group comments for max and min to say "iteratee" instead of "iterator" because I messed that up in my last PR.

This is the last PR in the Collections family.

… tests.

typescript-bot · 2020-07-26T06:48:13Z

@reubenrybnik Thank you for submitting this PR!

This is a live comment which I will keep updated.

Code Reviews

Because you edited one package and updated the tests (👏), I can help you merge this PR once someone else signs off on it.

Status

✅ No merge conflicts
✅ Continuous integration tests have passed
✅ Most recent commit is approved by type definition owners or DT maintainers

All of the items on the list are green. To merge, you need to post a comment including the string "Ready to merge" to bring in your changes.

Diagnostic Information: What the bot saw about this PR

{
  "type": "info",
  "now": "-",
  "pr_number": 46352,
  "author": "reubenrybnik",
  "owners": [
    "borisyankov",
    "jbaldwin",
    "ccurrens",
    "confususs",
    "jgonggrijp",
    "ffflorian",
    "regevbr",
    "peterblazejewicz",
    "reubenrybnik"
  ],
  "dangerLevel": "ScopedAndTested",
  "headCommitAbbrOid": "50f6f6c",
  "headCommitOid": "50f6f6c58c7c80ed2ddc274da254143e774b1077",
  "mergeIsRequested": true,
  "stalenessInDays": 0,
  "lastPushDate": "2020-07-27T23:28:05.000Z",
  "lastCommentDate": "2020-07-28T21:14:14.000Z",
  "maintainerBlessed": false,
  "reviewLink": "https://github.com/DefinitelyTyped/DefinitelyTyped/pull/46352/files",
  "hasMergeConflict": false,
  "authorIsOwner": true,
  "isFirstContribution": false,
  "popularityLevel": "Popular",
  "anyPackageIsNew": false,
  "packages": [
    "underscore"
  ],
  "files": [
    {
      "path": "types/underscore/index.d.ts",
      "kind": "definition",
      "package": "underscore"
    },
    {
      "path": "types/underscore/underscore-tests.ts",
      "kind": "test",
      "package": "underscore"
    }
  ],
  "hasDismissedReview": false,
  "ciResult": "pass",
  "lastReviewDate": "2020-07-28T19:31:15.000Z",
  "reviewersWithStaleReviews": [],
  "approvalFlags": 2,
  "isChangesRequested": false
}

typescript-bot · 2020-07-26T06:48:14Z

🔔 @borisyankov @jbaldwin @ccurrens @confususs @jgonggrijp @ffflorian @regevbr @peterblazejewicz — please review this PR in the next few days. Be sure to explicitly select Approve or Request Changes in the GitHub UI so I know what's going on.

typescript-bot · 2020-07-26T08:52:37Z

👋 Hi there! I’ve run some quick measurements against master and your PR. These metrics should help the humans reviewing this PR gauge whether it might negatively affect compile times or editor responsiveness for users who install these typings.

Let’s review the numbers, shall we?

Comparison details 📊

	master	#46352	diff
Batch compilation
Memory usage (MiB)	58.9	64.0	+8.6%
Type count	12020	12708	+6%
Assignability cache size	1461	1468	0%

Language service
Samples taken	6815	7272	+7%
Identifiers in tests	6815	7272	+7%
`getCompletionsAtPosition`
Mean duration (ms)	193.4	196.1	+1.4%
Mean CV	8.3%	7.6%
Worst duration (ms)	356.5	405.8	+13.8%
Worst identifier	_	stringMemo
`getQuickInfoAtPosition`
Mean duration (ms)	188.1	191.6	+1.9%
Mean CV	8.1%	7.9%	-3.2%
Worst duration (ms)	376.2	420.1	+11.7%
Worst identifier	extractChainTypes	extractUnderscoreTypes

It looks like nothing changed too much. I won’t post performance data again unless it gets worse.

reubenrybnik · 2020-07-27T09:19:29Z

As mentioned in #46306, there are some more exciting revisions to invoke that I now plan to make in a separate PR after #45201 is closed where I'll bump the TS version up as high as I can.

…By to EnumerableKey.

jgonggrijp

Another breeze!

types/underscore/index.d.ts

reubenrybnik · 2020-07-28T21:13:21Z

Thanks @jgonggrijp, this is the last PR for Collections! Just two more PRs for Arrays, one for your requested changes to Objects, and some cleanup 😁

reubenrybnik · 2020-07-28T21:14:14Z

Ready to merge

typescript-bot · 2020-07-28T21:49:42Z

I just published @types/underscore@1.10.16 to npm.

reubenrybnik · 2020-07-29T00:02:53Z

Darn, forgot to re-run the latest set of changes against my company's solution, with the switch to requiring a result of EnumerableKey I get one new error where an iterator in groupBy results in string | undefined. I think it's good for that to be an error though since we should probably filter out the undefined cases (or come up with a better default key) instead of ending up with a weird "undefined" key (or at least explicitly choose to end up with that).

@reubenrybnik

…rray Tests - SortBy, IndexBy, CountBy, and Invoke by @reubenrybnik * Updating type definitions for sortBy, indexBy, and countBy and adding tests. * Switching "iterator" to "iteratee" for a few test groups. * Making a few adjustments to groupBy to better match similar functions. * Updating type definitions for invoke and adding tests. * Constraining allowed iteratee results for groupBy, indexBy, and countBy to EnumerableKey.

@reubenrybnik

…rray Tests - SortBy, IndexBy, CountBy, and Invoke by @reubenrybnik * Updating type definitions for sortBy, indexBy, and countBy and adding tests. * Switching "iterator" to "iteratee" for a few test groups. * Making a few adjustments to groupBy to better match similar functions. * Updating type definitions for invoke and adding tests. * Constraining allowed iteratee results for groupBy, indexBy, and countBy to EnumerableKey.

reubenrybnik added 3 commits July 25, 2020 23:26

Updating type definitions for sortBy, indexBy, and countBy and adding…

738387b

… tests.

Switching "iterator" to "iteratee" for a few test groups.

8031e8c

Making a few adjustments to groupBy to better match similar functions.

7a3d48c

reubenrybnik requested a review from borisyankov as a code owner July 26, 2020 06:48

typescript-bot added Where is GH Actions? GH Actions didn't give a response to this PR Popular package This PR affects a popular package (as counted by NPM download counts). Author is Owner The author of this PR is a listed owner of the package. labels Jul 26, 2020

typescript-bot removed the Where is GH Actions? GH Actions didn't give a response to this PR label Jul 26, 2020

typescript-bot added the Perf: Same typescript-bot determined that this PR will not significantly impact compilation performance. label Jul 26, 2020

reubenrybnik mentioned this pull request Jul 27, 2020

[@types/underscore] Collection and Array Tests - Zip, Unzip, and Object #46306

Merged

8 tasks

Updating type definitions for invoke and adding tests.

a42dc68

reubenrybnik changed the title ~~[@types/underscore] Collection and Array Tests - SortBy, IndexBy, and CountBy~~ [@types/underscore] Collection and Array Tests - SortBy, IndexBy, CountBy, and Invoke Jul 27, 2020

Constraining allowed iteratee results for groupBy, indexBy, and count…

50f6f6c

…By to EnumerableKey.

jgonggrijp approved these changes Jul 28, 2020

View reviewed changes

types/underscore/index.d.ts Show resolved Hide resolved

typescript-bot added Owner Approved A listed owner of this package signed off on the pull request. Self Merge This PR can now be self-merged by the PR author or an owner labels Jul 28, 2020

typescript-bot merged commit a8dcb61 into DefinitelyTyped:master Jul 28, 2020

reubenrybnik mentioned this pull request Aug 6, 2020

[@types/underscore] More comprehensive tests for Underscore collection and array functions + definition changes as appropriate #45201

Closed

10 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[@types/underscore] Collection and Array Tests - SortBy, IndexBy, CountBy, and Invoke#46352

[@types/underscore] Collection and Array Tests - SortBy, IndexBy, CountBy, and Invoke#46352
typescript-bot merged 5 commits intoDefinitelyTyped:masterfrom
reubenrybnik:underscore-tests-sortBy-indexBy-countBy

reubenrybnik commented Jul 26, 2020 •

edited

Loading

Uh oh!

typescript-bot commented Jul 26, 2020 •

edited

Loading

Uh oh!

typescript-bot commented Jul 26, 2020

Uh oh!

typescript-bot commented Jul 26, 2020

Uh oh!

reubenrybnik commented Jul 27, 2020

Uh oh!

jgonggrijp left a comment

Uh oh!

Uh oh!

reubenrybnik commented Jul 28, 2020

Uh oh!

reubenrybnik commented Jul 28, 2020

Uh oh!

typescript-bot commented Jul 28, 2020

Uh oh!

reubenrybnik commented Jul 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

reubenrybnik commented Jul 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

typescript-bot commented Jul 26, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Code Reviews

Status

Uh oh!

typescript-bot commented Jul 26, 2020

Uh oh!

typescript-bot commented Jul 26, 2020

Uh oh!

reubenrybnik commented Jul 27, 2020

Uh oh!

jgonggrijp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

reubenrybnik commented Jul 28, 2020

Uh oh!

reubenrybnik commented Jul 28, 2020

Uh oh!

typescript-bot commented Jul 28, 2020

Uh oh!

reubenrybnik commented Jul 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

reubenrybnik commented Jul 26, 2020 •

edited

Loading

typescript-bot commented Jul 26, 2020 •

edited

Loading