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

[BUGFIX beta] Add private WeakMap #12990

Merged
merged 3 commits into from
Feb 20, 2016
Merged

[BUGFIX beta] Add private WeakMap #12990

merged 3 commits into from
Feb 20, 2016

Conversation

mmun
Copy link
Member

@mmun mmun commented Feb 20, 2016

This is a step towards fixing #12212. Finishes #12417 and #12224.

Implements a partial polyfill for WeakMap.

There is a small but important caveat. This implementation assumes that the weak map will live longer (in the sense of garbage collection) than all of its keys otherwise it is possible to leak the values stored in the weak map. In practice, most use cases satisfy this limitation which is why it is included in ember-metal.

let weak = this.writableWeak();
return weak[symbol] = value;
};

Copy link
Member Author

Choose a reason for hiding this comment

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

These aren't used...

@stefanpenner
Copy link
Member

Can we make this at least bugfix beta, as it will allow completion of the computed sort fix.

@mmun
Copy link
Member Author

mmun commented Feb 20, 2016

@stefanpenner Sounds good... should I rebase it down to three [BUGFIX beta] commits for you, Travis and me?

@krisselden
Copy link
Contributor

yay!

@mmun mmun changed the title Add private WeakMap [BUGFIX beta] Add private WeakMap Feb 20, 2016
@stefanpenner
Copy link
Member

@mmun sure

krisselden added a commit that referenced this pull request Feb 20, 2016
[BUGFIX beta] Add private WeakMap
@krisselden krisselden merged commit b00c076 into emberjs:master Feb 20, 2016
@mmun mmun deleted the weak-map branch February 21, 2016 00:04
@rwjblue rwjblue mentioned this pull request Apr 10, 2016
3 tasks
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.

4 participants