Add Weakmap – Weak from Map -> Obj, not Obj -> Map

[stefanpenner]

RFC is likely not needed, as this is a polyfil using embers internal machinery for a spec'd ES6 feature. We don't really have flex room on this feature, constraints are fixed.

• docs
  • basic API
  • flesh out + examples
• feature flags
• RFC WeakMap RFC rfcs#91

[rwjblue]

Do we have a need for this in the codebase right now?

[stefanpenner]

@rwjblue i believe the final fix for #12221 will want this

[stefanpenner]

after some thought, in theory we could let people choose the direction they want the weakness to be in...

[stefanpenner]

I'm going to likely merge this, and not make it public. Im going to need it for #12221

[rwjblue]

👍

[stefanpenner]

• error if primitive key is used:

a = new WeakMap()
WeakMap {}
a.set('a', '1')
VM266:2 Uncaught TypeError: Invalid value used as weak map key(…)

[tomdale]

Excited to have this. A public way to decorate objects with metadata without exposing __meta__ and friends === ❤️❤️❤️

[stefanpenner]

@tomdale got any comments for the RFC?

[mmun]

@stefanpenner The current implementation will retain values longer than expected:

function leak(key) {
  let weakMap = new WeakMap();
  weakMap.set(key, new VeryLargeObject());
}

let obj = {};
leak(obj);

// obj will retain the VeryLargeObject for the rest of its miserable life

EDIT:

I spoke briefly with @wycats and he convinced me that you should only be using a WeakMap if the weak map lives longer than its keys. In that case there is no GC problem. And that is also the required use case for the Ember.computed.sort bug fix.

[stefanpenner]

Ya that is expected. Userland can only make weak one sided (reasonably), this pattern is still valuable, as it essentially opens storage on meta as a stable public API.

I thought I documented it as such, but if you have suggestions on how to improve the docs to be clearer I'm all ears

[homu]

☔ The latest upstream changes (presumably #12990) made this pull request unmergeable. Please resolve the merge conflicts.

[mmun]

The commits in this PR were rebased and merged in another PR (#12990). There is still work required to make WeakMap public. Let's have that conversation in a separate PR and close this one.

@stefanpenner Gotcha. I tweaked the docs to make it a little clearer.