fixes #40: throw when map is mutated in getOrInsertComputed callbacks

[michaelficarra]

Fixes #40. Alternative to #71 (closes #71).

Other currently unexplored options:

1. Throw only if the value is different.
2. Lock the map before calling the callback and throw in any mutating operation called from within the callback.
   1. The difference here being that we could catch and prohibit any mutation rather than just the addition of the key we're currently setting.

[bakkot]

LGTM. TypeError seems not quite right, but we don't have a better one and it's not worth introducing a new error kind just for this I expect.

[jridgewell]

Why not just insert a { [[Key]]: _key_, [[Value]]: *undefined* } before invoking the callback, then mutate that record with its new value afterwards?

[bakkot]

The callback could remove and re-add the key, so that wouldn't be sufficient on its own. With how the spec machinery is written I guess you could check to see if the entry record still has the right [[Key]], but that doesn't actually match how implementations work under the hood.

I expect doing the additional map update would be more expensive for the common case of no mutation in the callback, so that doesn't seem worth it.

[zloirock]

It will make polyfilling of those methods too painful that could slowdown Map and WeakMap at all for a significant part of users for years.

[bakkot]

@zloirock The polyfill is literally just

Map.prototype.getOrInsertComputed = function (key, callback) {
  if (typeof callback !== 'function') throw new TypeError;
  if (this.has(key)) return this.get(key);
  let value = callback(key);
  if (this.has(key)) throw new TypeError;
  this.set(key, value);
  return value;
};

(except with intrinsic versions of get/set/has/TypeError, obviously)

There is no reason this should slow down anything else, nor be particularly slow itself. Maybe you are thinking of a different behavior than the one this is actually proposing? This isn't the "lock the map" thing, it's just checking to see if the key was inserted in the map again after calling the callback.

[zloirock]

This isn't the "lock the map" thing, it's just checking to see if the key was inserted in the map again after calling the callback.

Ah, in this case, it's fine - after the mentioned issue, I was sure that this option implies map locking that will be added in the future. Sorry -)

[dead-claudia]

Will note that my optimization note suggestion in #40 (comment) also applies here with little modification.