Skip to content
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

Allowing 'instance methods' to be importable #101

Closed
jakearchibald opened this issue Mar 8, 2018 · 26 comments
Closed

Allowing 'instance methods' to be importable #101

jakearchibald opened this issue Mar 8, 2018 · 26 comments

Comments

@jakearchibald
Copy link

jakearchibald commented Mar 8, 2018

Apologies if this has already been discussed to death.

I'm interested in solving this case:

import Whatever from "./Whatever.js";

const whatever = new Whatever();
whatever.commonThing(123);
whatever.rareAndComplexThing(321);

…where rareAndComplexThing adds a lot of weight to the Whatever class, and should ideally be an opt-in:

import Whatever from "./Whatever.js";
import { rareAndComplexThing } from './whatever-extras.js';

const whatever = new Whatever();
whatever.commonThing(123);
rareAndComplexThing(whatever, 321);

I don't really like the above. Maybe it's because it isn't clear that rareAndComplexThing may change the state of whatever.

An alternative is what RxJS did:

import Whatever from "./Whatever.js";
import "./whatever-extras/rareAndComplexThing.js";

const whatever = new Whatever();
whatever.commonThing(123);
whatever.rareAndComplexThing(321);

Where rareAndComplexThing.js imports Whatever.js and adds to its prototype. But this doesn't work well with tree-shaking. RxJS's pipable operators fix the issue for them, but it doesn't really work for the general case.

I got excited about the pipeline operator here, but now I've written it down, I'm not so sure:

import Whatever from "./Whatever.js";
import { rareAndComplexThing } from './whatever-extras.js';

const whatever = new Whatever();
whatever.commonThing(123);
whatever |> rareAndComplexThing(321); // F-sharp style
whatever |> rareAndComplexThing(, 321); // or Hack style

With the F-sharp style, rareAndComplexThing would have to be implemented like:

export function rareAndComplexThing(val) {
  return whatever => {
    // do stuff with whatever & val
  }
}

That overhead isn't needed with the Hack style, but it feels really clunky. Again, it isn't clear that the state of whatever may be altered. I think what I really want is just:

import Whatever from "./Whatever.js";
import { rareAndComplexThing } from './whatever-extras.js';

const whatever = new Whatever();
whatever.commonThing(123);
whatever::rareAndComplexThing(321);

…where rareAndComplexThing is called like rareAndComplexThing.call(whatever, 321). Even if pipeline ships, I think I'd rather go with:

import Whatever from "./Whatever.js";
import { rareAndComplexThing } from './whatever-extras.js';

const whatever = new Whatever();
whatever.commonThing(123);
whatever.do(rareAndComplexThing, 321);

Is the bind operator dead in favour of the pipeline operator? I like pipeline, but it doesn't feel like it solves this particular use-case. Is it intended to?

@jakearchibald jakearchibald changed the title Allowing 'instance' methods to be importable Allowing 'instance methods' to be importable Mar 8, 2018
@js-choi
Copy link
Collaborator

js-choi commented Mar 8, 2018

@jakearchibald: I’ll let @mAAdhaTTah speak for Proposal 1: F-sharp Style Only. From the perspective of Proposal 4: Smart Pipelines, the code above would look like:

import Whatever from "./Whatever.js";
import { rareAndComplexThing } from './whatever-extras.js';

new Whatever() |> do {
  #.commonThing(123);
  ::rareAndComplexThing(321);
}

The smart-pipelines explainer has a section addressing its relationship with the existing function-binding proposal ::. I’ll paste it below.

An existing proposal for ECMAScript function binding has three use cases:

  1. Extracting a method from an object as a standalone function:
    object.method.bind(object) as ::object.method.
  2. Calling a function as if it were a method call on an object:
    func.call(object, ...args) as object::func(...args)
  3. Creating a function by binding an object to it:
    func.bind(object) as object::func.

The smart-pipelines Core Proposal + Additional Feature PF [edit: mostly; see footnote] subsumes the ECMAScript function binding proposal in the first use case (prefix ::). But the other two use cases (infix ::) are not addressed by smart pipelines. Smart pipelines and infix function binding :: can and should coexist. In fact, infix function binding could be made more ergonomic in many cases by replacing prefix ::function with a shortcut for the expression #::function.

With smart pipelines

With existing proposal

Method extraction can be addressed by pipeline functions alone, as a natural result of their pipeline-operator-like semantics.
+> console.log is equivalent to $ => $ |> console.log, which is a pipeline in bare style. This in turn is $ => console.log($)

Promise.resolve(123).then(+> console.log)

…and $ => console.log($) is equivalent to console.log.bind(console).

Promise.resolve(123).then(console.log.bind(console))
Promise.resolve(123).then(::console.log)
$('.some-link').on('click', +> view.reset)
$('.some-link').on('click', ::view.reset)
const { hasOwnProperty } = Object.prototype
const x = { key: 5 }
x::hasOwnProperty
x::hasOwnProperty('key')

For terse method calling/binding, the infix :: operator would still be required.

const { hasOwnProperty } = Object.prototype
const x = { key: 5 }
x::hasOwnProperty
x::hasOwnProperty('key')

But the :: operator would only need to handle method calls. No operator overloading of :: for method extraction (that is, :: as a prefix operator) would be needed.

The function bind operator ::’s proposal switches its prefix form ::function to mean #::function instead of object.bind(object), then many nested functions would become even terser.

a(1, +>
  ::b(2, +> )
)

See block parameters for further examples.

a(1, $ =>
  $::b(2, $ => )
)

TL;DR: The smart-pipelines Core Proposal + Additional Feature PF subsumes the ECMAScript function binding proposal in the first use case (prefix ::). But the other two use cases (infix ::) are not addressed by smart pipelines. Smart pipelines and infix function binding :: can and should coexist. In fact, infix function binding could be made more ergonomic in many cases by replacing prefix ::function with a shortcut for the expression #::function.

new Whatever() |> do {
  #.commonThing(123);
  ::rareAndComplexThing(321);
}

Edit: There is a special type of secure/robust method pre-extraction that @ljharb desires to still be addressed: see #110 (comment) and #110 (comment). This would be orthogonal to the smart pipeline operator + feature PF; neither precludes the other.

@mAAdhaTTah
Copy link
Collaborator

I don't think we've discussed this at length, but @benlesh commented on its viability for RxJS here.

It's less clunky in F# if you make them both arrow functions:

const rareAndComplexThing = val => inst => {
  // do stuff
}

@zenparsing
Copy link
Member

Is the bind operator dead in favour of the pipeline operator? I like pipeline, but it doesn't feel like it solves this particular use-case.

I can't really speak for the vital signs of the bind operator, but it has significant issues:

  • The chaining part of it seems to be subsumed by pipeline.
  • There has historically been significant resistance to profligate this usage outside of class and object literal methods.
  • The syntax conflicts with more traditional uses of :: as a scope resolution or method reference (i.e. Java) operator.
  • The method extraction syntax is odd: it seems like extension methods and method extraction should be orthogonal instead of smooshed together with the same operator.

We still need a solution for method extraction, and it would be nice to have a better solution for extension methods, but I'm not sure that :: is it.

@rudiedirkx
Copy link

#101 (comment) looks like what with used to do. I thought we didn't like that.

@aikeru
Copy link

aikeru commented Mar 9, 2018

With had some key differences...
IE it would crawl each scope to find any matching property, whereas this is limited to a sigil that is currently invalid as a variable name and would only be usable within this scope... AFAI understand it.
(EDIT: this is better explained below by someone more knowledgeable)

@js-choi
Copy link
Collaborator

js-choi commented Mar 9, 2018

@rudiedirkx: The semantics of #101 (comment) and smart pipelines are very different than the semantics of with statements.

with statements create a lexical environment with an object Environment Record, which dynamically binds variables to the property names of some binding object—and shadowing any outer variables with the same names. This means that variable bindings cannot be determined until the binding object is initialized during runtime, and even then it can dynamically change. This compromises the static analyzability of variable scope, making security analysis and type analysis by JavaScript engines much more difficult.

In contrast, only declarative lexical environments may establish a topic binding. This topic binding is immutable, and its origin and its type are always statically analyzable. It is, basically, a tacit const binding, except the reference is a nullary operator rather than an identifier.

@spion
Copy link

spion commented Mar 20, 2018

There has historically been significant resistance to profligate this usage outside of class and object literal methods.

TC39 really just needs to start owning the behavior of this. Its out there, and it can be used outside of classes and object literals already. Heck its being used in the DOM like that and will probably stay around forever! Sticking our heads in the sand wont make it go away. this is as an implicitly passed argument and the bind operator would fill in the missing piece of syntax that allows you to pass that argument. It would probably resolve the this confusion once and for all (e.g. how does it get all these weird values in the DOM? Oh easy, they just pass a this argument)

@jakearchibald to make the pipeline operator less of a badly fitting lego brick you'd probably need https://github.com/tc39/proposal-partial-application

@mAAdhaTTah
Copy link
Collaborator

@spion That's true of the F# proposal; Smart pipelines does it's own type of partial application with its lexical topic.

@spion
Copy link

spion commented Mar 20, 2018

@mAAdhaTTah the smart pipeline proposal has terrible syntax costs. It introduces a whole new contextual lambda syntax just to remove x=>, and another additional lambda syntax to generalize the behaviour outside of the context of |>. It has a very bad cost-benefit ratio.

rbuckton's partial application syntax is at least non-contextual, and while arbitrary expressions are great, I'd rather stick to arrow functions than pay the above costs.

@hax
Copy link
Member

hax commented Mar 20, 2018

I think pipeline operator is very good, but it's not a good answer for extension methods or method extraction because the paradigms is very different (FP vs OO).

OO guys expect the calling of extension methods should look very close to normal methods. Pipeline fails in four small but important aspects:

  • Extension accessors is impossible in pipeline way. IMO, extension accessor is a secondary but still valid requirement, though current bind operator proposal is also short of it.
  • No parens in a |> b. This is weird for OO guys. And a |> b(c) make it even more inconsistent.
  • Operator precedence is far from . which OO guys would expect. And have spaces around |> in common coding style implies this difference. For example: !a |> b vs !a::b.
  • When you see [a, b, c] |> f, it's ok that f is a function to deal with a Tuple, but in the view of OO, f should be only a extension method for Array.

So the conclusion is very clear to me that we need both pipeline op and extension methods, and no one could replace the other.


About method extraction, the main motivation and criterion should be ergonomics, or we should just stick on arrow function (...args) => a.b(...args) (though in event listener case you need a little extra effort). I prefer infix operator like a.&b because prefix forms like :: a.b or +> a.b don't make much sense in ergonomics. Even they save the keystrokes, the main frustrations I observed is you only realize you need extract the method when you already enter a., then you have to move cursor backward.


There has historically been significant resistance to profligate this usage outside of class and object literal methods.

I totally agree @spion and I want to point out the difficulty of this is much exaggerated in the whole community and frighten the newcomers, which look similar to FUD of semicolons 8 years ago.

@jakearchibald
Copy link
Author

jakearchibald commented Mar 20, 2018

In terms of explaining this stuff to developers:

foo::bar(3);

Call bar with arguments 3 as if it were an instance method of foo.

// F-sharp style without partial application
foo |> bar(3); 

Call bar with arguments 3, which returns a function. Now call that function with arguments foo.

// Hack style (similar to F-sharp with partial application)
foo |> bar(, 3); 

Call bar with arguments foo (which replaces ) and 3.

I don't think :: is difficult to explain. I think the F-sharp method is weird to explain. The hack style is ok to explain.

However, I think it's more expected that an instance method would mutate an instance, whereas it's less expected that a non-instance method would.

I acknowledge that we already have methods on Object that mutate their argument, but I think in these cases we'd have put them on Object.prototype if it weren't for compatibility worries.

@mAAdhaTTah
Copy link
Collaborator

@spion No argument from me (I'm backing F#), but you do end up tying the F# proposal to the partial application proposal in order for it to approach the level of power offered by the Smart proposal.


@jakearchibald Explaining exactly what F#-style is doing like that sounds worse than explaining "you can import these extension methods and use them with the pipeline." Even better, using them with the pipeline operator looks like fluent APIs OO developers are already familiar with:

[1, 2, 3]
  |> filter(x => x > 1)
  |> map(x => x * 2)
  |> reduce((a, b) => a + b)

You don't necessarily need to explain all the details under the hood off the bat, and basically any FP-style library (Ramda, lodash/fp) works this way already, so you can treat them like array extension methods already.

Once this rolls out to the language proper, libraries with an OO approach can design their API around this feature, which gives OO developers an opportunity to tree-shake methods they don't use while still working with an API that should be fairly familiar to them.

The F#-style proposal allows the details to be hidden from OO developers like this, allowing them to think of these imported methods as just methods. The Smart pipeline puts their implementation details front-and-center, cuz they have to know that the method they just imported isn't a method, its a function that takes two arguments, one of which is the instance itself.

None of this is an argument against ::, as there are use cases for it that aren't solved by the pipeline (see #110 for those; method extraction is the big one), but I do think the use case of "importing methods" is better solved by F#-style pipeline than Smart pipelines or the bind operator.

@mAAdhaTTah
Copy link
Collaborator

@spion @hax Also, IRT this, I think you are reading too far into what he said. this isn't going anywhere, but its behavior can be confusing. We don't want to introduce new usages that make it even more confusing, so we do have to be careful as to how we interact with it in new proposals.

I don't think that resistance is a killer for the bind operator, nor do I think it means there will be no new this usages in the language; it's more a call to be cautious and thoughtful about how and why we're introducing new usages into the language.

@jakearchibald
Copy link
Author

jakearchibald commented Mar 20, 2018

@mAAdhaTTah

Explaining exactly what F#-style is doing like that sounds worse than explaining "you can import these extension methods and use them with the pipeline."

Sure, but you need to know that stuff if you ever need to write your own, or debug an existing one.

you can treat them like array extension methods already

Yeah, it's ok for methods that don't mutate the original, but take:

arrayLike |> reverse();

It isn't clear to me that arrayLike would be mutated. Especially if I understand that reverse() returns a function and arrayLike is passed to that function.

arrayLike::reverse();

If my understand is that reverse is called as an instance method of arrayLike, mutation is much more expected.

@mAAdhaTTah
Copy link
Collaborator

Sure, but you need to know that stuff if you ever need to write your own, or debug an existing one.

Of course; I'm mostly focusing on the experience of developer consuming these APIs, not the ones writing them. That said, I don't think writing the arrow function as I mentioned above is complex, and debugging them is still straightforward, and in general, I think writing a library requires you to know more about the details than consuming said library. The F# proposal pushes the complexity to the library author rather than consumer, which I think is generally appropriate.

It isn't clear to me that arrayLike::reverse(); mutates the array either because a userland implementation may not necessarily do that. In fact, I would be more likely to expect the opposite in this particular case; otherwise, I would just use native reverse. In general though, I wouldn't rely on an operator to mean mutation or not in either case because it depends on the semantics of the implementation you're importing.

@jakearchibald
Copy link
Author

It isn't clear to me that arrayLike::reverse(); mutates the array

To clarify, I mean that it's more expected that foo::bar() may mutate foo, whereas it's less expected with bar(foo) or foo |> bar.

Like I said earlier, there's Object.assign(a, b) which does this already, but I think these were compromises for what should have been instance methods.

@mAAdhaTTah
Copy link
Collaborator

I understood what you meant; I don't have that expectation. I think both of those cases are reliant on the APIs you're looking at. jQuery as the prime fluent API example has both; addClass mutates the instance whereas find returns a new instance. $('.my-element') |> addClass('hello') looks like it mutates the instance because of how I understand the semantics of addClass, not because of the choice of operators.

Object.assign was not a compromise; it follows the convention set by extend methods implemented in jQuery, Lodash, underscore and other utility libraries. That's more a "pave the cow path". Similarly, there are far more functions out there that operate on parameters passed into them than there are functions that operate on this, so the pipeline operator fits better into that ecosystem than the bind operator for this particular use case. The places where I've seen importable extension methods implemented (e.g. RxJS) does so in way compatible w/ the pipeline operator, not bind.

@spion
Copy link

spion commented Mar 20, 2018

I think you are reading too far into what he said. this isn't going anywhere, but its behavior can be confusing. We don't want to introduce new usages that make it even more confusing, so we do have to be careful as to how we interact with it in new proposals.

How about introducing new usages that make it less confusing?

The infix bind operator would make all this use cases seem less special. Of course you can call various bits of code with whatever this argument you want:

  • actual method calls with prototype chain: o.name(...args) <=> let fn = findInPrototypeChain(o, 'name'); o::fn(...args)
  • DOM event handler call site desugaring: someElement::eventHandlerCode(event).
  • new operator desugaring: new constructor(...args) <=> Object.create(constructor.prototype)::constructor(...args) (well with a special case for no return values)
  • setTimeout(x.fn, delay) <=> setTimeout(findInPrototypeChain(x, 'fn'), delay) - fn is not called, so this isn't passed, only found in the prototype chain, then given as an argument. Then setTimeout doesn't have o to call o::fn() with.

Also prefix bind fits like a glove there: I want to let setTimeout do o::fn(), so I need to partially apply o to it, using ::o.fn

I realise JS wants to hang around with the cool FP languages, but please, lets think a little bit more about what makes the most sense.

@mAAdhaTTah
Copy link
Collaborator

Part of the problem currently is ::foo.bar !== ::foo.bar, which actually makes its usage for DOM event handlers difficult, and the solution is not entirely clear / straightforward (see tc39/proposal-bind-operator#46). Other aspects of what you want (e.g. method extraction for your 4th bullet point) can still be solved by the bind operator. There isn't anything about this proposal that precludes you from pushing for :: to do so; @ljharb will not allow pipeline to advance without a solution for method extraction at least on the table.

That said, pipeline is not intended solely for FP usage; my whole argument above is that it's great for OOP developers because you can design and implement fluent APIs without needing to ship everything on the prototype, allowing everyone to build smaller bundles, while also enabling FP-like usages as well (something the bind operator cannot do).

TC39 is introducing new usages for this; there are a number of proposals currently in discussion related to class and method binding, which are intended to make it less confusing, and the partial application proposal has a suggestion for ?this.method, intended to allow the receiver to be applied later. Throw in the bind operator itself for method extraction, and that bundle of features is certainly not reflective of TC39 backing away from this.

@ljharb
Copy link
Member

ljharb commented Mar 20, 2018

Note that it's only a problem for event handlers if you use it inline; for robustness, you'd need to always use it at module level, and thus, store it in a variable.

@hax
Copy link
Member

hax commented Mar 20, 2018

My comment may be off topic.

@mAAdhaTTah

Also, IRT this, I think you are reading too far into what he said.

I don't think so.

this isn't going anywhere, but its behavior can be confusing.

I don't think this is more confusing than any other features if you use a feature in wrong way. Unfortunately I find some books just list all the behaviors of this in one place, which is totally wrong teaching method for the newcomers.

Even that, I don't see much problems of confusion caused by this in real world. Note, there are many bugs caused by this, but that does not mean this is confusing, programmers understand this well, they just made mistakes. What the real problem is we are short of language features which could provide protection of such mistakes. I would like to write a proposal which might help for discover the mistakes earlier next week.

We don't want to introduce new usages that make it even more confusing, so we do have to be careful as to how we interact with it in new proposals.

It's weird to me that why we never worry about the fp style features like pipelines would be too strange to many js programmers, but consider a feature related to this which already in JS from the first day would be "even more confusing".

Note, I love pipeline operators. (though some extension of smart pipelines proposal seems problematic, for example it's too easy to lose the context of what current # mean, maybe just the examples are not good enough, I'm not sure.) I just think we can have both world of OO and FP.

@js-choi
Copy link
Collaborator

js-choi commented Mar 20, 2018

TC39 is introducing new usages for this; there are a number of proposals currently in discussion related to class and method binding, which are intended to make it less confusing, and the partial application proposal has a suggestion for ?this.method, intended to allow the receiver to be applied later. Throw in the bind operator itself for method extraction, and that bundle of features is certainly not reflective of TC39 backing away from this.

I know you’re already aware of this @mAAdhaTTah, but I do want to note, for the benefit of other readers, that smart pipelines with Additional Feature PF also already addresses this use case. Using the example from tc39/proposal-partial-application#23:

const arrayToStringArray = ?this.map(x => "" + x);
…would instead be:
const arrayToStringArray = +> #.map(x => "" + x); 

Smart pipelines with Additional Feature PF would completely subsume @rbuckton’s partial-application proposal, as far as I can tell. But even with Additional Feature PF, subsume are orthogonal to both method binding and @ljharb’s use case for secure method pre-extraction (#110 (comment)). Both smart pipelines and method binding/pre-extraction can coexist, such that:

const arrayToStringArray = ?this.map(x => "" + x);
inputs |> ?this.map(x => "" + x) |> console.log;

…would instead be:

const arrayToStringArray = +> #.map(x => "" + x); 
inputs |> #.map(x => "" + x) |> console.log;

this coexists with smart pipelines fine enough, because this is just another expression for topic style to use.


[@hax] Note, I love pipeline operators. (though some extension of smart pipelines proposal seems problematic, for example it's too easy to lose the context of what current # mean, maybe just the examples are not good enough, I'm not sure.) I just think we can have both world of OO and FP.

I’m interested in your feedback. I’ve done my best to make the lexical scoping of # clear; it’s an explicit goal of the proposal, and it is my hope that the real-world examples bear that and the other goals out.

That’s not on topic for this issue, though. Please feel free to open another issue either here (specify Proposal 4) or in the smart-pipelines proposal. I’m busy still working on the spec right now, but I would definitely be listening.

@tabatkins
Copy link
Collaborator

Closing this issue, as the proposal has advanced to stage 2 with Hack-style syntax.

This solves @jakearchibald's original use-case; you just have to write ordinary functions, and then you get the readability benefits of method-chaining.

@jakearchibald
Copy link
Author

I don't feel it solves my use-case, but I don't think this proposal is the one I'm after.

@ljharb
Copy link
Member

ljharb commented Sep 9, 2021

I think the bind operator, or something like it, is more the solution you’re looking for. Watch the agenda for the next meeting.

@tabatkins
Copy link
Collaborator

I don't feel it solves my use-case, but I don't think this proposal is the one I'm after.

Well, it lets you import "methods" and call them on objects without resorting to function-call nesting, given you similar "linear code" benefits to method calls themselves while allowing for tree-shaking. In this respect, it solves the use-case as well as any pipe operator would.

If the use of the this parameter is particularly important for semantic reasons, and you want the syntax to reflect that, then it doesn't, yeah, but definitely keep watch out for the bind operator's revival. A this-based pipeline operator was never seriously considered. (It was discussed, but had sufficient problems as a general-purpose pipeline operator that nobody ever tried to champion it.)

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

10 participants