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

Use Map shim with _.uniq #2108

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

jridgewell
Copy link
Collaborator

Rounding out my PRs for today: An alternative implementation of #2099, using a Map shim. Performs just as well, but simplifies _.uniq instead of making it more complex.

This also paves the way for using a Map in _.memoize (#1862).

@megawac
Copy link
Collaborator

megawac commented Mar 13, 2015

This also paves the way for using a Map in _.memoize (#1862).

This won't be possible (by default) as theres no reasonable way to shim a WeakMap so in older environments (such as node 0.10) we'll end up creating some potentially severe memory leaks

@megawac
Copy link
Collaborator

megawac commented Mar 13, 2015

Why are you using Map instead of Set here?

See 2ce6f62

@jridgewell
Copy link
Collaborator Author

This won't be possible (by default)

It's just a Map, not a WeakMap. But this'll allow dev's to use a WeakMap if they want.

Why are you using Map instead of Set here?

I wanted to kill two birds with one stone. And, this is a direct port of #2099.

@jgonggrijp
Copy link
Collaborator

I think this would be the right thing to do in principle, but I hope it can be done in a way that takes up less space.

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.

4 participants