tc39 / proposal-upsert

ECMAScript Proposal, specs, and reference implementation for Map.prototype.upsert
https://tc39.es/proposal-upsert/
MIT License
204 stars 14 forks source link

`emplace` cannot be correctly and incrementally polyfilled #51

Closed conartist6 closed 2 years ago

conartist6 commented 2 years ago

The problem is that since emplace is defined not to call get and set, it can only be correctly polyfilled when completely reimplementing Map from scratch, as core-js does. In other words, it is impossible to build the functionality correctly by extending the Map prototype because you would not have the requisite privileged access to the underlying data structure.

My impression is that that would make this one of those features that we can all use in ~5 years when everyone is finally using engines that natively support it.

ljharb commented 2 years ago

What makes it a Map imo isn’t the api, it’s the internal slots - thus, if your subclass isn’t utilizing them, it’s not really a Map instance, it’s just a Map-like.

iow, there’s no need to subclass Map at all for such a class.

conartist6 commented 2 years ago

I agree, but my question is: how will you (you personally probably) write a shim? You won't have access to the internal slots you need to avoid calling get and set.

zloirock commented 2 years ago
const has = Function.call.bind(Map.prototype.has);
const get = Function.call.bind(Map.prototype.get);
const set = Function.call.bind(Map.prototype.set);

function emplace(key, handler) {
  const M = this;
  if (has(M, key)) { // incl. RequireInternalSlot(M, [[MapData]])
    let value = get(M, key);
    if ('update' in handler) {
      value = handler.update(value, key, M);
      set(map, key, value);
    } return value;
  }
  const inserted = handler.insert(key, M);
  set(M, key, inserted);
  return inserted;
}

Object.defineProperty(Map.prototype, 'emplace', { writable: true, configurable: true, value: emplace });
conartist6 commented 2 years ago

Wouldn't it have to be:

class MapWithEmplace {
  constructor(entries) {
    this.#nativeMap = new NativeMap(entries);
  }
  get(key) {
    this.#nativeMap.get(key);
  }
   // ... 
  emplace(key, handler) {
    const M = this.#nativeMap;
    if (has(M, key)) {
      let value = get(M, key);
      if ('update' in handler) {
        value = handler.update(value, key, M);
        set(map, key, value);
      } return value;
    }
    const inserted = handler.insert(key, M);
    set(M, key, inserted);
    return inserted;
  }
}
zloirock commented 2 years ago

Wouldn't it have to be

Why?


In this case, the problem is not in writing shims - the problem is in subclassing and generic behavior. But TC39 kills it already for a long time.

conartist6 commented 2 years ago

The proposal currently says (it should state this in clearer language) that the implementation of map.emplace must not call map.get or map.set. Yours does. You can make it not do that by using internal slots, but you can only have access to internal slots because you replaced native Map with something else (global.Map = ...).

zloirock commented 2 years ago

It does not call map.get or map.set, you could remove them at all. It works completely by the spec and interacts with internal slots.

conartist6 commented 2 years ago

How does it interact with the internal slots? If I write new Map right now, I certainly can't get access to its internal slots.

zloirock commented 2 years ago

Please, take a look at the polyfill source above.

conartist6 commented 2 years ago

I don't feel at all confident that you are hearing what I am saying : (

The polyfill source you link implements Map.prototype.emplace in terms of Map.prototype.has etc, which AFAIK is wrong. Do you disagree? Did I misread?

conartist6 commented 2 years ago

See: https://github.com/tc39/proposal-upsert/issues/47

zloirock commented 2 years ago

One more time - it does not call .get or .set on this, they are unbound. It has no any observable difference with direct interaction with internal slots. It works completely by the spec of this proposal.

conartist6 commented 2 years ago

How?? There's something going on here that's extremely obvious to you and not at all obvious to me

I know you'll have access to the internal slots of you've completely implemented Map from the ground up.

A shim however is not a ground up implementation, but only an implementation of the part of the functionality that is missing.

zloirock commented 2 years ago

Please, write any example that will not work correctly by the spec with the polyfill above.

conartist6 commented 2 years ago

What is RequireInternalSlot?

zloirock commented 2 years ago

has checks it.

conartist6 commented 2 years ago

I'm going to take 24 hours to cool off from this. Thanks.

conartist6 commented 2 years ago

My question is still utterly unanswered.

ljharb commented 2 years ago

@conartist6 call-binding is how you unobservably base a polyfill’s functionality off of existing builtins, but either way, polyfillability is not a consideration of spec proposals.

zloirock commented 2 years ago

but either way, polyfillability is not a consideration of spec proposals

Why do you think so? Polyfill is something that significantly improves user experience and allows you to start using a feature a few years earlier. If polyfillability can be implemented at a low cost, it should be implemented.

conartist6 commented 2 years ago

@ljharb Thanks for the succinct clarification. I've never needed to use that pattern so I didn't recognize it.

ljharb commented 2 years ago

@zloirock it’s not that it’s my preference, it’s a statement of fact about the committee’s historical priorities. Here’s not the place to debate it.

zloirock commented 2 years ago

@ljharb this is a thread about polyfillability, so it's the correct place for this discussion. Historical priorities show that almost all that can be made polyfillable without any significant issues is made polyfillable - and this is something that should be defined in TC39 rules because someone is trying to change it.

conartist6 commented 2 years ago

If you want to make a rule it seems reasonable to start a thread in a place where the people who will be affected (and who can effect rule changes) would see it.

This is a thread based on a technical misunderstanding, and I am closing it.