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

BREAKING(collections): deprecate groupBy() #3663

Merged
merged 6 commits into from
Nov 22, 2023

Conversation

NotWoods
Copy link
Contributor

groupBy can be replaced by Object.groupBy in Deno 1.37. Creating this as a draft for now because the TypeScript lib isn't updated, but running this without typechecking passes fine.

@lino-levan
Copy link
Contributor

Related to #3594.

@iuioiua iuioiua changed the title Deprecate groupBy BREAKING(collections): deprecate groupBy() Nov 2, 2023
collections/group_by.ts Outdated Show resolved Hide resolved
@iuioiua iuioiua marked this pull request as ready for review November 2, 2023 03:48
@iuioiua iuioiua requested a review from kt3k as a code owner November 2, 2023 03:48
Copy link
Collaborator

@iuioiua iuioiua left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Types for Object.groupBy() and Map.groupBy() were added in Deno v1.38.

This change looks reasonable and good to me. Thanks!

@kt3k
Copy link
Member

kt3k commented Nov 2, 2023

Deprecation makes sense to me, but safari still doesn't support this according to MDN https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/groupBy

I'm afraid we might get similar reaction to the case of readableStreamFromIterable() #3579 (comment)

@lino-levan
Copy link
Contributor

Not sure about this. I'm still considering std/shim or std/ponyfill or something.

@github-actions github-actions bot removed the testing label Nov 22, 2023
@kt3k
Copy link
Member

kt3k commented Nov 22, 2023

We updated the deprecation policy in #3807

Now we recommend the users to pin the URL to the old std version if they still need them.

Copy link
Member

@kt3k kt3k left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@kt3k kt3k merged commit 7f5229e into denoland:main Nov 22, 2023
12 checks passed
@NotWoods NotWoods deleted the deprecate-group-by branch November 23, 2023 21:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants