tc39 / proposal-call-this

A proposal for a simple call-this operator in JavaScript.
https://tc39.es/proposal-call-this/
MIT License
122 stars 5 forks source link

Counter proposal: Object.methodsToFunctions() #2

Closed theScottyJam closed 3 years ago

theScottyJam commented 3 years ago

For context, I'll be heavily relying on the pipeline operator proposal here.

I know this proposal is in its very early stages, and the details aren't even finalized, but I've been trying to also think about the use case for the bind operator, and trying to see if there's perhaps a better way to solve this problem. And, I think I found an interesting solution.

Let's start by saying we have a new Object.methodToFunction() function (whether we actually add this or not can be debated, but it makes for a good stepping stone to what comes next). We can define it roughly as follows:

Object.methodToFunction = function (method) {
  // In the actual definition, it won't be suseptable to `delete Function.prototype.call` issues.
  return (thisArg, ...args) => method.call(thisArg, ...args)
}

// Usage

const map = Object.methodToFunction(Array.prototype.map)
;[2, 3]
  |> map(^, x => x + 1)
  |> console.log(^) // [3, 4]

Are we on the same page thus far? Object.methodToFunction will basically just take a method that's intended to be called with a this arg, and turn it into a function who's this arg becomes the first parameter.

OK, now let's define Object.methodsToFunctions():

Object.methodsToFunctions = function (prototype) {
  return Object.entries(prototype)
    |> ^.map(([key, value]) => [key, Object.methodToFunction(value)])
    |> Object.fromEntries(^)
}

// Usage example #1

const { map, filter } = Object.methodsToFunctions(Array.prototype)

;[2, 3]
  |> map(^, x => x + 1)
  |> filter(^, x => x % 2 === 0)
  |> console.log(^) // [4]

// Usage example #2

const $Array = Object.methodsToFunctions(Array.prototype)

;[2, 3]
  |> $Array.map(^, x => x + 1)
  |> $Array.filter(^, x => x % 2 === 0)
  |> console.log(^) // [4]

Now that's pretty nice, isn't it? It fully protects against prototype mutations, while providing people with a very simple-to-use API. I could see people wanting to use this even if they don't care about prototype mutations.

ljharb commented 3 years ago

We have that already for a single method - Function.call.bind(Array.prototype.map). I don't think it would be a good idea to have a "make a clone of the object" approach.

The problem is that, still, someone has to rely on an API method existing - Object.methodToFunction in this case. The need is for a syntactic form of .call and/or .bind.

theScottyJam commented 3 years ago

We have that already for a single method - Function.call.bind(Array.prototype.map). I don't think it would be a good idea to have a "make a clone of the object" approach.

Function.prototype.call.bind() is a neat trick that lets you bind different versions of call() for different instances upfront, but it can't be used dynamically later on with an unknown this value. Isn't that one of the major issues being solved? (e.g. this can't be coded in a robust way: export const myBind = (f, thisValue) => f.bind(thisValue))

Object.methodsToFunctions() will even allow you to turn Function.prototype.call() into something that can be dynamically used later on, in a robust way.

const $Function = Object.methodsToFunctions(Function.prototype)

// later...

myFunction
  |> $Function.call(^, 2, 3)

The problem is that, still, someone has to rely on an API method existing - Object.methodToFunction in this case. The need is for a syntactic form of .call and/or .bind.

The difference is Object.methodsToFunctions is static, and does not rely on being bound to anything in particular, unlike Function.prototype.call. If you want to use it robustly later on, you can just pick it off of Object from the start, and it'll work.


If the primary objective of this proposal is to make it easy and ergonomic to capture the state of things when your library first loads, then only use the captured values, then I think this fits the bill very nicely.

ljharb commented 3 years ago

Effectively you're proposing a static Function.callBind method (which is where i'd prefer to place it, and what i'd prefer to call it), which is "fine", but not as nice as a syntactic approach.

theScottyJam commented 3 years ago

Effectively you're proposing a static Function.callBind method

Yeah, pretty much.

which is "fine", but not as nice as a syntactic approach.

I actually find it nicer than the "->" syntax 🤷‍♂️️ - compare these examples:

/*** "->" syntax ***/
// With namespacing
const $Array = { ...Array.prototype }
;[2, 3] |> ^->($Array.map)(x => x + 1)

// Without namespacing
const { map } = Array.prototype
;[2, 3] |> ^->map(x => x + 1)

/*** Object.methodsToFunctions() ***/

// With namespacing
const $Array = Object.methodsToFunctions(Array.prototype)
;[2, 3] |> $Array.map(^, x => x + 1)

// Without namespacing
const { map } = Object.methodsToFunctions(Array.prototype)
;[2, 3] |> map(^, x => x + 1)

When we're not using namespaces, that ^->map(x => x + 1) does look pretty sweet, but it's really not any more verbose than |> map(^, x => x + 1). However, if I were to use this feature, I think I would always do so in a namespaced manner, so I don't keep having to change what I'm destructuring from prototypes each time I want to use another method - I don't know how many others would be that way. If that happens, then I think the function does a much better job than the syntax: |> $Array.map(^, x => x + 1) vs |> ^->($Array.map)(x => x + 1)

ljharb commented 3 years ago

I'm not sure what you mean by "namespacing" here.

theScottyJam commented 3 years ago

Sorry, that was pretty unclear :p

I just meant I was putting all of the methods from Array.prototype into a "namespace" I was calling $Array (the namespace just being an object - a space to group those method names together), instead of putting each function within Array.prototype into the environment as a stand-alone function. I should have used some other word to distinguish these two scenarios.

ljharb commented 3 years ago

That's totally something you're free to do, but it's not something I do in any of my hundreds of packages that would benefit from the bind operator. That unnecessarily makes a new object and copies N methods, when I might only need 1 - to me that sounds like the same performance downsides as React.createClass auto-binding all component methods, which React thus moved away from.

theScottyJam commented 3 years ago

Hmm, if performance is a concern, then perhaps there's ways for the browser to skirt around that in its implementation. For example, I understand that the browser doesn't pre-create the entire globalThis object, rather, it lazily loads things in when they're needed. Similarly, the object returned by Object.methodsToFunctions() perhaps doesn't need to copy everything up-front. Perhaps it's possible to use similar techniques where functions are lazily added to the object on an as-needs basis. This kind of laziness will probably get invalidated as soon as someone tries to mutate a prototype somewhere.

Does that make any sense? I don't really know anything about browser optimization, but I assume if performance really is a concern here, there might be ways to help mitigate the performance issues. Maybe.

At the very least, it should be possible for the browser to optimize a scenario like this:

const { map } = Object.methodsToFunctions(Array.prototype)

It should be possible to notice that "map" is the only method being extracted, so no need to create a bunch of temporary methods if they're all going to be tossed on the next step.

ljharb commented 3 years ago

Some browsers lazy load globals, many do not. I wouldn’t be so quick to assume or rely on optimizations.

Either way, it’s a philosophical concern - making/importing/loading more than you need is icky.

theScottyJam commented 3 years ago

I guess it's not good to rely on that behavior either if it's not mandatory. I'll keep thinking about this.

theScottyJam commented 3 years ago

hmm.

I guess this would be possible if a pick syntax ever got through.

const $Array = Array.prototype.{ map, filter } |> Object.methodsToFunctions(^)

But alas, that's currently not in any plans.

I'll keep thinking.

theScottyJam commented 3 years ago

I'm actually going to run with my precious comment as a valid solution. If we're willing to spend a little syntax budget on this issue, and if Object.methodsToFunctions() is just as good or arguably better than the current proposal when combined with a pick syntax, then perhaps this is a better route to go.

Plus, we would get the beloved pick syntax :)

Of course the new function and the pick syntax should be different proposals, but together they would make a pretty nice team.

ljharb commented 3 years ago

To be clear, I think I wouldn't be the only one opposed to an API that tries to copy an object (not something generally done in JS builtins), has to make intelligent decisions like "only pick the own functions, not inherited or non-functions", and then mutates them. It reminds me of bluebird's promisifyAll, which has a ton of problems and is generally considered to be a poor solution.

js-choi commented 3 years ago

Taking a look at methodsToFunctions—it reminds me kind of some aspects of @hax’s extensions proposal.

Would it address generic-method APIs? I’m planning to add a section to this proposal about generically transferrable methods like Array.from that use their this binding to perform actions. For example, ArrayLike->(Array.from)(items). Ah, wait, yes, it would—it would just make the this binding the generated functions’ first arguments. $from(ArrayLike, items).

theScottyJam commented 3 years ago

@ljharb I do think you're less likely to run into issues than the kind you'd find with bluebird's promisifyAll. Prototypes generally don't get to crazy - they're usually just an object with methods. On the other hand, it's expected that promisifyAll() would be used on all sorts of different types of objects.

But still, point taken. You may still run into the occasional issue with prototypes, and the new function is designed to work on non-prototype objects as well.

ljharb commented 3 years ago

As an example, Array.prototype has Symbol.unscopables and length which aren't functions, and it has one symbol method (all of its properties are non-enumerable, at least).

theScottyJam commented 3 years ago

Actually, I think it wouldn't be that bad. The copied object isn't intended to behave exactly the same as the source object. It can just copy over the methods only (no symbol properties, etc). This means it won't copy over Symbol.unscopables, nor does it need to. It also means it won't copy over getters (The current proposal doesn't support getters either).

There is, however, the issue of own vs inherited properties you brought up. I think the only logical way to proceed would be to have it also copy all inherited properties, which means all resulting objects would be receiving a modified copy of functions like toString. Otherwise, turning an inheritance relationship from MyThing -> Object to MyThing -> MyBaseThing -> Object would be a breaking change to anyone using this new method on the MyThing class.

Copying and modifying things like toString() everytime someone wishes to use Object.methodsToFunctions() would certainly be anoying, and something I don't like. That just doesn't feel right.

ljharb commented 3 years ago

"No symbol properties" means it won't copy Symbol.iterator, which is a pretty important method.

Doing anything "deep" is a bit of a code smell and also simply not how JS operates.

theScottyJam commented 3 years ago

In the current proposal, how do you deal with Symbol.iterator? Wouldn't it have to be something awkward like this?

const { [Symbol.iterator]: iterate } = Array.prototype
for (const x of [1, 2, 3]->iterate) { ... }

I think it would be just as awkward with what I'm proposing:

// This is assuming renaming is allowed in the pick syntax. Otherwise you just have to pick it off a different way
const $Array = Array.prototype.{ [Symbol.iterator]: iterate } |> Object.methodsToFunctions(^)
for (const x of [1, 2, 3] |> iterate(^)) { ... }

I guess this point doesn't matter that much though, because...

Doing anything "deep" is a bit of a code smell and also simply not how JS operates.

Agreed. I might just close this issue if I can't find any way to revive it.

js-choi commented 3 years ago

In the current proposal, how do you deal with Symbol.iterator? Wouldn't it have to be something awkward like this?

const { [Symbol.iterator]: iterate } = Array.prototype
for (const x of [1, 2, 3]->iterate) { ... }

Yeah, or eventually:

const iterate = getIntrinsic('Array.prototype[Symbol.iterator]');
for (const x of [1, 2, 3]->iterate) { ... }

…Though, well, I don’t remember how getIntrinsic is supposed to handle Symbol.iterator right now. It’d make it more ergonomic, whatever way.

theScottyJam commented 3 years ago

Well, thanks for the discussion. I think we've hit a dead end here, so I'll go ahead and close this.