Split the proposal

[bmeck]

I've had a few talks yesterday and this proposal may be split up with a new disparate set of goals:

• Get a default value if the key does not exist in the map and, importantly, DO NOT INSERT THE VALUE (see FAQ on why insertion without update is something I consider extremely undesirable)
• Perform an update on the value for a key that already exists in the Map. ( @tabatkins / @bmeck )

Each of these would preserve:

• No incomplete values are inserted into the map ( @bmeck )
• The value returned would always be of the same type as the values in the Map ( various )

It also satisfies the plenary conflicts:

• Separate methods means no object to pull props off of. ( see arguments about this value )
• Order of callbacks cannot be confused if each only takes 1 callback. ( secondary feedback )
• Does not alter the existing semantics of .get(key), by requiring an argument for creating a default value it uses a different method signature as a means of opt-in ( @bmeck )

It compromises on:

• Insertion workflow is still sub optimal, but you can always get an existing/default value and .set() regardless of if it is already present (see example with current methods).
• pauses such as yield or await leave a potential for using stale values if the key is inserted during the pause, after the default is retrieved, but before a set() is performed.

I think such a set of discrete APIs moves the idea of inserting into the map out of the scope of the proposal due to conflicts about co-location of insert/update but we could just leave it to users to document workarounds like what ruby documents for default_proc where a default initializer performs the set eagerly:

map.get(key, () => {
  const value = ...;
  map.set(key, value);
  return value;
})

[ljharb]

For the default value, what about providing it via an options bag to the constructor, with the same mechanism collection normalization uses?

[bmeck]

@ljharb

Normalizing via the constructor has issues with:

Does not alter the existing semantics of .get(key)

Same for .has, also call sites are generally how you populate things fully so there is also a concern about conflict with:

No incomplete values are inserted into the map

as well since the default value may necessarily be incomplete without call site data. For some maps that insert primitives this is likely a lesser issue and per plenary people thought a different proposal for such constructor level hooks for data exfiltration does not directly conflict with methods like what was presented, nor a modified .get signature.

[ljharb]

Every use case I’ve had for a Map having a default value (and the way map default values work in Ruby) is entirely known at the construction site; I’m very confused why you’d need this kind of overload at the call site if the capability already existed at construction. Can you elaborate on those use cases?

[bmeck]

@ljharb anything that is not directly derived from the key. E.G. modified from the readme:

// data model requires the person have an email, not derived from the key
const contact = contacts.insert(key, () => ({email: null}));

Still, that constructor level instrumentation can be done as a separate proposal and has an increased scope due to having to deal with other data exfiltration like .has / .delete / the contract modification of .get (if it even happens via .get [defaultdict in python for example does not]).

[tabatkins]

Every use case I’ve had for a Map having a default value (and the way map default values work in Ruby) is entirely known at the construction site; I’m very confused why you’d need this kind of overload at the call site if the capability already existed at construction. Can you elaborate on those use cases?

As I said during the f2f, I have examples of both using defaultdict and .setd() in my Python code - most of it is defaultdict, but not all. We absolutely should not treat the two approaches as in conflict - they're both useful. @bmeck provides the general distinction - anytime I need to derive the default value from information beyond the key itself, I need to use .setd(); if I can derive it just from the key, I probably want to use defaultdict.

---

Back to the OP, you're not explicit about what APIs you're proposing, but I think it's:

• .get(key, defaultFn) - returns the value for the key if it exists, otherwise returns the result of executing defaultFn()
• .update(key, fn) - if the key exists, call fn(key) and set the result back into the map; if the key doesn't exist, throw.

And losing the ability to easily say "if the key exists, return its value; otherwise, call this fn and set its result to the key, then return it".

Is this right?

[bmeck]

@tabatkins yes, those 2 APIs but no commitment to names.

[tabatkins]

I'd be sad to lose the "get, or else set to this default and return" case (aka python's setdefault).

[bmeck]

@tabatkins with the default value callback you can perform an eager .set inside the default, I think that satisfies the use case even if it lacks some ergonomics.

[tabatkins]

Ah, I was assuming that the cb would be called with no arguments, like the defaultdict cb is in Python. But part of the ergonomics in Python is that constructors are called as plain functions, so just passing list works fine; in JS you'd have to wrap the constructor call in an arrow function anyway, so there's very little reason to not pass in the key for cases that want to use it.

So yeah, that satisfies me.

[dominictarr]

This is trivial to implement in userspace. for example: https://github.com/choudharyhimanshu/map-with-default https://github.com/cGuille/default-map

[ljharb]

@dominictarr and both of those examples break with Map.prototype.set.call, Object.getPrototypeOf(Map.prototype, 'size').get.call, etc. This proposal makes it be a real Map, and the base prototype methods will work on it.

[dominictarr]

why does it matter if it's a "real" map? second one uses composition of an inner Map, so it will return the correct values. someone wanting this feature will get it using the second module. (the first just uses an object, but it's logic looks correct)

[ljharb]

@dominictarr it matters because coding robustly, and caching call-bound Map.prototype methods at module require time, will throw because they lack an internal slot.

The second one still fails, because it doesn't use extends and so it does not get the internal slots of a Map.

[dominictarr]

I don't get it. what isn't "robust" about using composition? anyway your application is using composition when you use a map. Personally if I needed defaults on a map i'd just do map.has(key) ? map.get(key) ; default_value given there a only a couple of those modules (and they arn't very popular) I'd suspect that's what most are doing if they need this.

I don't understand what you mean by

caching call-bound Map.prototype methods and they lack an internal slot.

are you talking about some sort of optimization? can you link me to anything about this?

presumably if they did use extend it would have these properties?

[ljharb]

I’m talking about making my module robust against someone later modifying or deleting Map.prototype methods. I use the same technique in most all my packages for most every instance method, rather than calling them directly.

[bmeck]

Modifying Map.prototype.get might be non-viable given `[].map(myMap.get.bind(myMap)) would get multiple arguments and likely not do what people want. CC: @codehag

[ljharb]

That's a risk with adding arguments to any function - and I don't believe that's typically been considered a barrier to language evolution.

[tabatkins]

Not to mention it's both simpler and shorter to spell that code [].map(x=>myMap.get(x)), so the presence of explicit bind-ing code like that is, I suspect, vanishingly small.

[ljharb]

Also, the parseInt problem is well-known, and it's a best practice to use inline callbacks for callback-taking methods, for this exact reason - to make it quite clear that there aren't any unexpected arguments being passed.

[codehag]

Not to mention it's both simpler and shorter to spell that code [].map(x=>myMap.get(x)), so the presence of explicit bind-ing code like that is, I suspect, vanishingly small.

I think we can verify this. I guess the question that I was pulled in to answer was the viability and my response there would be to check webcompat for it. Alternatively -- in the spec we can check if the second argument to see if it is well formed. .bind in the above case passes an int and an array as the next two args.

[bmeck]

We have proposals like bind and extensions that would increase the surface area that could make problematic forms more attractive. I'm not sure stating that it is less inconvenient would affect other things like code generators so they could still create such code for w/e reason.

[bergus]

Not sure if the proposal is back to the drawing board at this stage but I'd like to throw in Haskell's (containers) Data.Map as prior art. I do especially love insertWith and alter whose functionality I'm missing in JavaScript.

Translating them to the JS Map api, I'd propose

• .setWith(key, newValue, (oldValue, newValue, key, map) => updatedValue)) where the "merge" callback is called if an entry for the key already exists, with old value and new value as arguments, computing the value to replace the existing one. Otherwise it just inserts the new value.
• .alter(key, (oldValueOrUndefined, key, map) => newValueOrUndefined). This is the most generic operation available for updating maps. It offers value insertion, update and deletion all in one go. Given JavaScript's lack of a native Option/Maybe type it would need to fall back to undefined to signify "no existing value" or "entry should be removed", but I think that would be ok in any use cases of maps I've seen so far (and follows the behaviour of the get method).

[dead-claudia]

@bergus Your .setWith is equivalent to .emplace if we define newValue to be a thunk to match Haskell's lazy semantics.

Map.prototype.setWith = function (key, newValue, update) {
  const value = newValue()
  return this.emplace(key, {
    insert: () => value,
    update: oldValue => update(oldValue, value, key, map)
  })
}

I like your idea for alter, though I would prefer returning some sort of sentinel rather than undefined so you can still handle undefined in generic contexts (less surprising for library authors). Also, the key and map parameters are redundant - let's just keep it to maybe old value to maybe new value.