tc39 / proposal-partial-application

Proposal to add partial application to ECMAScript
https://tc39.es/proposal-partial-application/
BSD 3-Clause "New" or "Revised" License
1.02k stars 25 forks source link

confusing use of `new` with the partial application #11

Closed royling closed 3 years ago

royling commented 7 years ago

Using new with the partial apply syntax in the below example is a little bit confusing:

function f(x, y) { this.z = `${x}, ${y}` }
const g = new f("a", ?); // <-- a little bit confusing here
const obj = g(1); // creates an f instance
obj.z; // 'a, 1'

Commonly, new makes a constructor call, it creates an instance. What about moving new to the generated partial function (or constructor) like below?

const G = f('a', ?)
const obj = new G(1) // though this may be unintuitive, it seems like creating a G instance
haltcase commented 7 years ago

I'm not sure I'd go for supporting partial application with constructors. Especially with ES2015+ classes where it makes even less sense since you can't call them as functions at all. Feels like the only other way to allow it is if new was required at both sites, which I think I like even less.

royling commented 7 years ago

IMO, ES2015 class is just syntax sugar, it does not change the fact that the constructor is just a function. And from my understanding, this partial application is eventually a high order function, which transforms a function's signature:

const partial = (originalFn) => partialFn

Hence, it's not necessary to distinguish a constructor from regular functions.

haltcase commented 7 years ago

@royling but they're not purely syntax sugar, as in you can't accomplish all the same things in both. You can't call a class as a function - so it is necessary to differentiate them. new is required if you're creating an instance of an ES2015+ class.

function Foo (...args) {
   if (!(this instanceof Foo)) {
     return new Foo(...args)
  }
}

Foo() // works just fine
class Foo {}

Foo() // -> TypeError: Class constructor Foo cannot be invoked without 'new'
royling commented 7 years ago

@citycide you are right on the point of syntax sugar. I updated my comment above.

My point is this partial application acts as a high order function, it's not necessary to handle regular functions and constructors differently. So, coming to the syntax, no need to add new on the partial application:

// Given class Foo:
class Foo {
  constructor(x, y) { }
}

const Bar = Foo(?, 0);

const baz = new Bar(1); // will it be more intuitive to use new here?
haltcase commented 7 years ago
const Bar = Foo(?, 0);

I'm wary that this would give people the impression that classes are callable when they're not. Requiring new at both sites is the only way to really avoid that misconception I think.

I'd still like to hear others' opinions but I'm mostly against this personally.

royling commented 7 years ago

Understood you, but bear in mind that the partial application syntax is not actually a call expression. For a regular function const g = f(?, 0), it's identical to:

const g = x => f(x, 0); // it's a function expression!

IMHO, we should think of the partial application for a class as below, to be consistent:

const Bar = (Foo) => class Partial extends Foo {
  constructor(x) {
    super(x, 0)
  }
}
jasmith79 commented 7 years ago

I agree with @citycide and see no advantage to implicitly papering over the requirement of new. If you really want to call class constructors as functions you can use any number of third-party solutions in conjunction with this proposal. To wit here's a quick decorator:

const applyConstructor = (ctor, args) => {
  return new (ctor.bind.apply(ctor, [ctor, ...args]));
};

class Foo {};

const doesntNeedNew = applyConstructor(Foo);
const foo = doesntNeedNew(); // no error
foo instanceof Foo; // true

There's also ramda's construct and various other methods of getting around it. I don't see why it has to be baked in to this proposal.

rbuckton commented 5 years ago

This will probably be determined by what we think makes sense for the semantics of partial application:

(A) If partial application behaved like Function.prototype.bind, you would not need a partially-applied new and would instead new the partially-applied function result:

class C { ... }

const f = C(1, ?); // ok
new f(2); // ok
f(2); // TypeError: Class constructor C cannot be invoked without 'new'

// transposed 'f':
const f = (() => {
  const fn = C;
  const p0 = 1;
  return fn.bind(null, p0); // NOTE: not strictly equivalent
})();

(B) If partially applied functions behave like an arrow function, you cannot new an arrow function and we would need to partially apply the new:

class C { ... }
const f = new C(1, ?); // ok
new f(2); // TypeError: f is not a constructor
f(2); // ok

// transposed 'f':
const f = (() => {
  const fn = C;
  const p0 = 1;
  return a0 => new fn(p0, a0);
})();

(C) A hybrid approach would allow new in both places:

class C { ... }
const f = C(1, ?); // ok
new f(2); // ok
f(2); // TypeError: Class constructor C cannot be invoked without 'new'

const g = new C(1, ?); // ok
new g(2); // ok
g(2); // ok

// transposed 'f':
const f = (() => {
  const fn = C;
  const p0 = 1;
  // note: behaves like Function.prototype.bind:
  const result = function (a0) {
    // 'new' not explicitly specified
    return new.target === undefined
      ? Reflect.apply(fn, this, [p0, a0])
      : Reflect.construct(fn, [p0, a0], new.target);
  };
  // note: fixup name and prototype
  Object.defineProperty(result, "name", {
    configurable: true,
    enumerable: false,
    writable: false,
    value: `bound ${fn.name}`
  });
  Object.setPrototypeOf(result, Object.getPrototypeOf(fn));
  result.prototype = fn.prototype;
  return result;
})();

// transposed 'g':
const g = (() => {
  const fn = C;
  const p0 = 1;
  // note: behaves like Function.prototype.bind:
  const result = function (a0) {
    // 'new' explicitly specified
    return Reflect.construct(fn, [p0, a0], new.target);
  };
  // note: fixup name and prototype
  Object.defineProperty(result, "name", {
    configurable: true,
    enumerable: false,
    writable: false,
    value: `bound ${fn.name}`
  });
  Object.setPrototypeOf(result, Object.getPrototypeOf(fn));
  result.prototype = fn.prototype;
  return result;
})();

Personally, I'm in favor of (C) as it is the most consistent with fewer user footguns.

rbuckton commented 3 years ago

As of #49 (along with some other recent commits), new is now permitted with partial application. The approach differs from all options above, but is closest to (C):

class C {}

var g;
g = C~();
g(); // throws because `C` was not partially applied with `new`
new g(); // throws because `C` was not partially applied with `new`

g = new C~();
g(); // ok: produces new C
new g(); // ok: produces new C (based on existing semantics for constructors returning objects).

g.name; // bound C
g.length; // 0
Object.getPrototypeOf(g) === C; // true
ljharb commented 3 years ago

Is g() instanceof g?

rbuckton commented 3 years ago

It should be, yes. I'm using BoundFunctionCreate and modified [[Call]] for bound function exotic objects in a way that (hopefully) that should be the case:

https://tc39.es/proposal-partial-application/#sec-bound-function-exotic-objects-call-thisargument-argumentslist

ljharb commented 3 years ago

It seems strange that you have to new a bound function, but you don’t have to new a partially applied function. (meaning, you have to use the “new” backwards - at binding time instead of invocation time)

rbuckton commented 3 years ago

The goal of syntactic PFA is to take an expression like this:

const inst = new C(x, y)

And allow you to defer it to fill in a portion later:

const f = new C~(x, ?)
const inst = f(y);

If you wrote C(x, y), for a class C it would throw, so const f = C~(x, ?) would be an odd transformation. There's nothing preventing you from writing this:

const f = new C~(x, ?);
const inst = new f(y);

Though the 2nd new is redundant.

Yes, this differs from bind, but it also makes more sense syntactically. bind is actually fairly strange in this regard when you compare it to call and apply (though unavoidable since there's no bindConstruct):

class C {}
C.call(null); // throws
C.apply(null, []); // throws
C.bind(null); // ok

So, honestly I think it makes more sense. Its a small semantic change to be more flexible that we could address in Stage 2 if necessary.

ljharb commented 3 years ago

I don't think it would be odd; C.bind(x) is newed later, and that's fine.

I think that calling a function, and getting something that's an instanceof the function, is very strange, and is something the language has explicitly moved away from with the introduction of class. I'm not really sure why instantiation is included in this proposal at all, tbh - partial application is about functions, not classes.

rbuckton commented 3 years ago

I can eventually make it support both:

const f = new C~();
new f(); // ok
f(); // ok

const g = C~();
new g(); // ok
g(); // error

But syntactically, the first option seems more correct to me.

rbuckton commented 3 years ago

Classes are functions, and having partial application not work with them seems strange. That would be like disallowing new C(...args) just because it uses new.

I want to be able to partially apply constructors. If we don't have new C~(), then I'd at least like to see Function.prototype.new so I could at least do C.new~(). Otherwise I'm back to needing a utility method:

const construct = (ctor, ...args) => new ctor(...args);
const g = construct~(C, ?);
const inst = g(x);
rbuckton commented 3 years ago

TBH, it would nice to have a parallel for construction like we do for .call . Reflect.construct doesn't work for this case because it takes an array as a 2nd argument so you can't use it with partial application:

Function.prototype.construct = function construct(newTarget = this, ...args) {
  return Reflect.construct(this, ...args, newTarget);
};

function F() {}
F.bind; // binds call/construct
F.call; // calls
F.construct; // constructs

// somewhat redundant now that we have ..., but:
F.apply; // calls w/arglist array
ljharb commented 3 years ago

Classes are constructor functions, which require new. A partially applied function that requires new should also require new.

rbuckton commented 3 years ago

Classes are constructor functions, which require new. A partially applied function that requires new should also require new.

Adapting a constructor to be used with a callback is frustrating and requires an arrow function. One of the reasons I want to have const f = new C~() not require new is to support this use case:

class Box {
  value;
  constructor(value) {
    this.value = value;
  }
}

[1, 2, 3].map(Box); // throws
[1, 2, 3].map(new Box~(?)); // [Box { value: 1 }, Box { value: 2 }, Box { value: 3 }]

Partial application returns a function that essentially acts as a mechanism to satisfy the missing arguments of the expression and return the result. Something like const g = new C~() essentially transposes into const g = function () { return new C(); }, but sets name and [[Prototype]] like Function.prototype.bind because I'm using a bound function exotic object. The fact that new works at all with the result is a byproduct of the fact that bound function exotic objects have both a [[Call]] and a [[Construct]]:

function f(x) { return x; }
class C { constructor(x) { this.x = 1; } }

// bind
const g1 = f.bind(null, 1);
g1(); // 1 - expected
new g1(); // f {} - instance of 'f' because 1 is not an object

const g2 = f.bind(null, { a: 2 });
g2(); // { a: 2 } - expected
new g2(); // { a: 2 } - the passed in argument because { a: 2 } is an object

const g3 = C.bind(null, 1);
g3(); // throws
new g3(); // C { x: 1 }

// partial application
const g4 = f~(1);
g4(); // 1 - expected
new g4(); // f {} - instance of 'f' because 1 is not an object

const g5 = f~({ a: 2 });
g5(); // { a: 2 } - expected
new g5(); // { a: 2 } - the passed in argument because { a: 2 } is an object

const g6 = C~(1); // classes have a [[Call]] that throws, so passes the IsCallable test
g6(); // throws
new g6(); // C { x: 1 }

const g7 = new C~(1);
g7(); // C { x: 1 } - We already supplied `new` so we shouldn't need to repeat ourselves
new g7(); // C { x: 1 } - `new` doesn't really matter since its the [[BoundFunction]] that gets created
ljharb commented 3 years ago

I don't see why [1, 2, 3].map(new Box~(?)); is better than [1, 2, 3].map(x => new Box(x));, considering that .map passes extra arguments - if Box is expecting more arguments, that would be the same footgun that arrow functions avoid.

rbuckton commented 3 years ago

I don't see why [1, 2, 3].map(new Box~(?)); is better than [1, 2, 3].map(x => new Box(x));, considering that .map passes extra arguments - if Box is expecting more arguments, that would be the same footgun that arrow functions avoid.

Here's another (if contrived) example:

class Box {
  constructor(x, set) {
    this.value = x;
    set.add(this);
  }
}

let set;
[1, 2, 3].map(new Box~(?, set = new Set());
set.length; // 3, set assigned once

[1, 2, 3].map(x => new Box(x, set = new Set());
set.length; // 1, set overwritten each time

[1, 2, 3].map(x => new Box(x, set ??= new Set());
set.length; // 3, but `??=` had to be evaluated 3 times.

One of the features of PFA is avoiding re-evaluating side-effects in the argument list.

ljharb commented 3 years ago

Or this one?

const set = new Set();
[1, 2, 3].map(x => new Box(x, set));

I understand this example is contrived, but i am skeptical there’ll be many, if any, use cases where using PFA is the cleanest approach.

Having an arguments list that performs side effects is what I’d argue is the antipattern, and we shouldn’t be endorsing that by introducing syntax that provides an affordance for it (PFA has other benefits, ofc)

rbuckton commented 3 years ago

Still somewhat contrived, but I should have made the example conditional:

const maybeArray = Math.round(Math.random()) ? [1, 2, 3] : undefined;

let set;
maybeArray?.map(new Box~(?, set = new Set());
set?.length; // undefined or 3, set assigned once

I've added the semantics around new to my slides to make sure they're covered during plenary.

ljharb commented 3 years ago

and why is that better than:

const maybeArray = Math.round(Math.random()) ? [1, 2, 3] : undefined;

let set = maybeArray?.length >= 0 && new Set();
maybeArray?.map(x => new Box(x, set);
set?.length; // undefined or 3, set assigned once

?

rbuckton commented 3 years ago

The problem with contrived examples is that they're easy to poke holes in.

Consider instead:

const maybeIterable = Math.round(Math.random()) ? generator() : undefined;

let set; // can't check length without exhausting generator
         // spreading into array *and* mapping below could exhaust heap
maybeArray?.map(new Box~(?, set = new Set());
set?.length; // undefined or number, set assigned once

In any case, there's nothing preventing you from doing const BoundBox = Box~(?); new BoundBox() which is the same semantics as .bind. But unlike .bind, we can leverage syntax to make the new part of the partial application:

const BoundBox = Box~(?);
new BoundBox(); // didn't new 'Box', so 'BoundBox' requires 'new'

const g = new Box~(?);
g(); // 'Box' applied with 'new', so 'g' doesn't require 'new'.

Here's another example to consider:

// no placeholders. callee and all arguments fixed. `new` also fixed:
const thunk = new ComplexObjectGraph~(dependency1, dependency2); // prepare construction
...
const result = thunk(); // finish construction

In both the g and the thunk cases, I don't see the point in repeating new if its already part of the partial application.

Finally, an additional benefit to partial new is how References are treated:

const BoundBox = Foo.~Box(?); // partial call preserves reference, holds `Foo` in memory (preventing GC).

const g = new Foo.~Box(?); // partial new doesn't need to preserve reference. `Foo` could theoretically be GC'd
ljharb commented 3 years ago

While you’re right about the problem with contrived examples, i don’t think any example that uses an assignment in expression position is going to be compelling; that’s widely considered to be a bad practice anyways.

The GC benefit is interesting, but requires a namespaced constructor (a pattern the committee recently, unfortunately, decided to avoid for builtins) so i think it might be uncommon.

In the thunk cases, you can use an arrow function as long as you aren’t reassigning the constructor name.

I think in general i remain very unconvinced that PFA is a pattern that will make sense at all with construction.

js-choi commented 3 years ago

but requires a namespaced constructor (a pattern the committee recently, unfortunately, decided to avoid for builtins)

Actually, I’d be quite interested to see where this resolution was made. Was it at the previous plenary?

ljharb commented 3 years ago

I think so, or the one before? It was when i wanted ArrayBuffer.Resizable instead of ResizableArrayBuffer, and separate from the consistency argument with SAB, some felt that it would always be confusing to have a namespaced constructor, if I’m remembering/inferring correctly.

dead-claudia commented 3 years ago

@ljharb The GC argument could be invalidated simply by doing something like let {Box} = Foo and then just doing x => new Box(x). Yes, it's an extra variable, but how often are you actually doing this in practice outside persistent globals (and other similar things like module namespaces) that won't likely ever be collectable anyways? Most likely, very rarely.

rbuckton commented 3 years ago

I think so, or the one before? It was when i wanted ArrayBuffer.Resizable instead of ResizableArrayBuffer, and separate from the consistency argument with SAB, some felt that it would always be confusing to have a namespaced constructor, if I’m remembering/inferring correctly.

Namespaced constructors are already in the spec (Intl), and more are on the way (Temporal). I don't think the decision around ResizableArrayBuffer is a standing policy. I think that was more about nested constructors.

ljharb commented 3 years ago

You might be right.

Either way, I still don't see the value over an arrow function. Partial functions are an FP thing, not an OO thing.