-
Notifications
You must be signed in to change notification settings - Fork 14
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
emplace
cannot be correctly and incrementally polyfilled
#51
Comments
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. |
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 |
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 }); |
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;
}
} |
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. |
The proposal currently says (it should state this in clearer language) that the implementation of |
It does not call |
How does it interact with the internal slots? If I write |
Please, take a look at the polyfill source above. |
I don't feel at all confident that you are hearing what I am saying : ( The polyfill source you link implements |
See: #47 |
One more time - it does not call |
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 A shim however is not a ground up implementation, but only an implementation of the part of the functionality that is missing. |
Please, write any example that will not work correctly by the spec with the polyfill above. |
What is |
|
I'm going to take 24 hours to cool off from this. Thanks. |
My question is still utterly unanswered. |
@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. |
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. |
@ljharb Thanks for the succinct clarification. I've never needed to use that pattern so I didn't recognize it. |
@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. |
@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. |
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. |
The problem is that since
emplace
is defined not to callget
andset
, it can only be correctly polyfilled when completely reimplementingMap
from scratch, as core-js does. In other words, it is impossible to build the functionality correctly by extending theMap
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.
The text was updated successfully, but these errors were encountered: