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

feat(collections): Add associatewith #1213

Merged
merged 6 commits into from
Sep 12, 2021
Merged

Conversation

getspooky
Copy link
Contributor

Builds a new Record using the given array as keys and choosing a value for each key using the given selector.

import { associateWith } from "https://deno.land/std@$STD_VERSION/collections/mod.ts";
import { assertEquals } from "https://deno.land/std@$STD_VERSION/testing/asserts.ts";

const names = ["Kim", "Lara", "Jonathan"];
const namesToLength = associateWith(names, (it) => it.length);

assertEquals(namesToLength, {
  "Kim": 3,
  "Lara": 4,
  "Jonathan": 8,
});

Related #1173

Copy link
Contributor

@LionC LionC left a comment

Choose a reason for hiding this comment

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

Thanks for the implementation! I left some comments :-)

collections/associate_with.ts Outdated Show resolved Hide resolved
collections/associate_with.ts Show resolved Hide resolved
collections/associate_with_test.ts Show resolved Hide resolved
@LionC LionC mentioned this pull request Sep 8, 2021
44 tasks
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.

@getspooky Nice work! LGTM

@kt3k
Copy link
Member

kt3k commented Sep 9, 2021

I'll wait a while before merging to see if there's any inputs, opinions on this because this item looks comparatively new in the proposed list.

@getspooky
Copy link
Contributor Author

I think something went wrong with test :

error: TS2551 [ERROR]: Property 'Signal' does not exist on type 'typeof Deno'. 'Deno.Signal' is an unstable API. Did you forget to run with the '--unstable' flag, or did you mean 'signal'?

@kt3k
Copy link
Member

kt3k commented Sep 9, 2021

@getspooky That CI error is going to be resolved in #1211, but we can't merge that fix right now because that will break stable CI. So let's just ignore canary CI for now.

@LionC
Copy link
Contributor

LionC commented Sep 12, 2021

@kt3k can we merge this before the upcoming release?

@kt3k
Copy link
Member

kt3k commented Sep 12, 2021

@LionC Thank you for the ping! Let's merge it.

@getspooky Thanks again for the work!

@kt3k kt3k merged commit 3ffd9fc into denoland:main Sep 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants