Skip to content
This repository has been archived by the owner on Jan 26, 2022. It is now read-only.

Other useful common functions #17

Closed
rbuckton opened this issue Oct 15, 2021 · 43 comments
Closed

Other useful common functions #17

rbuckton opened this issue Oct 15, 2021 · 43 comments
Labels
enhancement New feature or request

Comments

@rbuckton
Copy link

In https://github.com/esfx/esfx/blob/master/packages/fn/src/common.ts I've gathered a bunch of useful one-off functions for FP-style development. If we are considering adding .constant and .identity, perhaps some of those make sense as well. There are at least two that might be worth adding to the standard library:

  • noop() — Always returns undefined.
    • This is essentially the same as Function.prototype, though it might be useful to have a separate copy.
  • lazy(factory, ...args) — Creates a function with the following properties:
    • When invoked for the first time, calls factory with the supplied arguments and returns the result
    • Any subsequent invocation returns the result of the first invocation.

There's a bunch of other ones in there, but most are very niche:

  • incrementer(start = 0, step = 1) => () => start += step
  • decrementer(start = 0, step = 1) => () => start -= step
  • propertykey => object => object[key]
  • propertyWriterkey => (object, value) => object[key] = value
  • invoker(key, ...args) => (object, ...rest) => object[key](...args, ...rest)
  • caller(...args) => (func, ...rest) => func(...args, ...rest)
  • allocater(...args) => (ctor, ...rest) => new ctor(...args, ...rest)
  • factory(ctor, ...args) => (...rest) => new ctor(...args, ...rest)
  • uncurryThisFunction.prototype.bind.bind(Function.prototype.call)
  • complementf => (...args) => !f(...args)
  • both(a, b) => (...args) => a(...args) && b(...args)
  • either(a, b) => (...args) => a(...args) || b(...args)
  • fallback(a, b) => (...args) => a(...args) ?? b(...args)
@celmaun
Copy link

celmaun commented Oct 15, 2021

+1 for uncurryThis, I think that actually is used way more in the wild (in diff. variations) than any of the other proposed helpers.

@ljharb
Copy link
Member

ljharb commented Oct 15, 2021

@samhh
Copy link

samhh commented Oct 15, 2021

There's a lot of prior art for noop and it's something I've seen used in almost every JS project. For reference in fp-ts it's called constVoid.

@celmaun
Copy link

celmaun commented Oct 15, 2021

@ljharb do you have a problem with this entire proposal?

@celmaun
Copy link

celmaun commented Oct 15, 2021

There are like a half dozen bind proposals, they keep getting binned.

@ljharb
Copy link
Member

ljharb commented Oct 15, 2021

Of course i don’t. I think this is too large a list to be reasonable to add, and i think each addition will need its own motivation, and i don’t think there’s much value in an explicit noop when Function.prototype and ()=>{} exist.

@rbuckton
Copy link
Author

Like I said in the OP, most of those are niche and I wouldn't expect them to be accepted (at least, not on Function). And I mentioned that noop is essentially Function.prototype.

I mentioned noop because it's more obvious than Function.prototype. I doubt most JS devs would even consider using Function.prototype as a no-op callback. The problem with () => {} is that its yet another object allocation and there's no reference equality. With a clearly-labeled, built-in noop you can write branching code paths that optimize for cases where the callback is unused without resorting to cb?.(). The same is true for a built-in identity function:

function * tap(iterable, cb) {
  if (typeof cb !== "function") throw new TypeError("Function expected: cb");
  if (cb === Function.noop) {
    yield* iterable;
    return;
  }
  for (const x of iterable) {
    cb(x);
    yield x;
  }
}

function * map(iterable, cb) {
  if (typeof cb !== "function") throw new TypeError("Function expected: cb");
  if (cb === Function.identity) {
    yield* iterable;
    return;
  }
  let i = 0;
  for (const x of iterable) {
    yield cb(x, i++);
  }
}

iterable
  |> tap(^, debugMode ? value => console.log(value) : Function.noop)
  |> map(^, addIndex ? (value, index) => ({ ...value, index }) : Function.identity)

The one I have used most frequently, though, is lazy:

const getResult = Function.lazy(complexComputation);
getResult(); // performs computation, returns result
getResult(); // returns result

@rbuckton
Copy link
Author

Also, uncurryThis isn't really necessary if Partial Application advances, since you could then just write f.call~(...), but its a bit more obvious.

@ljharb
Copy link
Member

ljharb commented Oct 15, 2021

fwiw i do like the idea of an API solution for it, even if PFA or the bind operator advances, but i wouldn't call it "uncurry" since "currying" is a confusing concept.

@samhh
Copy link

samhh commented Oct 15, 2021

I'd argue currying is not confusing, merely unfamiliar. I've not worked with anyone who was unable to adapt to it even if, owing to current mainstream idioms, it's not what they were used to.

@ljharb
Copy link
Member

ljharb commented Oct 15, 2021

I'm quite familiar and I still find the concept confusing. Either way, "unfamiliar" is pretty much the same as "confusing" when it comes to language design.

@samhh
Copy link

samhh commented Oct 15, 2021

"Confusing" implies it's inherently hard to reason about whereas something merely unfamiliar may become comfortable and easy given a little time. It's an important distinction given users will be to varying extents unfamiliar with any new language construct.

@js-choi
Copy link
Collaborator

js-choi commented Oct 15, 2021

I’ve used complement, lazy (basically a memoization function), and n-ary versions of either and both (some-fn and every-pred in Clojure).

(To be honest, I’ve never used a specific noop function before in all my FP life. At most I might have done constant(null) or something. But I see that lodash.noop gets 400,000 NPM downloads weekly, so that’s something—though it’s difficult for me to see from its dependencies why people actually use lodash.noop.)

So I can see the appeal of many of these (particularly the complement and memoization functions). But @ljharb is right: the Committee will demand specific use cases and evidence of high frequency for each of these functions.

So I think a prerequisite for each of these functions (at least if we want it to have any chance in the Committee) would be to show that it’s been implemented in userland as one or more very popular libraries, along with specific use cases.

I also want to avoid this initial proposal becoming overly big at once. If the Committee shows that one of the proposal’s functions risks killing the entire proposal, then I will drop it. That goes for identity and for constant. Any controversial or complicated helper function can be punted to a future proposal.

complement and any/either maybe could be added if there are popular libraries that demonstrates existing demand for their use cases.

A memoization function might be complicated enough to warrant its own proposal.


I am uncertain whether including a call-bind/uncurry-this function would affect the chances of the resurrected bind-this operator that I will also propose to the Committee at the upcoming plenary meeting. I suspect that it might, which makes me hesitant to include it in this proposal.

@celmaun
Copy link

celmaun commented Oct 16, 2021

@js-choi Regarding your footnote, I think it would be on the contrary regarding affecting the chances. From my observation, for a syntax level change (+ competing proposals), you're looking at a good couple years to Stage 3. If Function.uncurryThis [1] gets there first (which is likely), then you can simplify your bind proposal greatly by saying it's basically syntactic sugar for Function.uncurryThis and complain about how verbose Function.uncurryThis is hehe. Then your bind proposal gets acceptance easy-peasy?

1: Agree about "curry" being confusing, so +1 for a better name. It's an annoying trend right now, functional programming terms are getting force-fed to the JS community.

@zloirock
Copy link

zloirock commented Oct 17, 2021

uncurryThis is npmjs.com/call-bind

Awesome alternative to Function.prototype.bind.bind(Function.prototype.call).

image

@zloirock
Copy link

Function.uncurryThis could be really useful.

@js-choi js-choi added the enhancement New feature or request label Oct 18, 2021
@js-choi
Copy link
Collaborator

js-choi commented Oct 18, 2021

[…] i don’t think there’s much value in an explicit noop when Function.prototype and ()=>{} exist.

@ljharb: I’m hemming and hawing about noop, because apparently _.noop and $.noop truly are used a lot, in a lot of very popular libraries. Examples include:

// From three@0.133.1/test/benchmark/benchmark.js
SuiteUI.prototype.run = function() {
  this.runButton.click = _.noop;
  this.runButton.innerText = "Running..."
  this.suite.run({
    async: true
  });
}

// From corejs-typeahead@1.3.1/test/bloodhound/transport_spec.js
for (var i = 0; i < 5; i++) {
  this.transport.get('/test' + i, $.noop);
}
expect(ajaxRequests.length).toBe(2);

// From corejs-typeahead@1.3.1/src/bloodhound/lru_cache.js
// “if max size is less than 0, provide a noop cache”
if (this.maxSize <= 0) {
    this.set = this.get = $.noop;
}

// From corejs-typeahead@1.3.1/src/bloodhound/bloodhound.js
sync = sync || _.noop;
async = async || _.noop;
sync(this.remote ? local.slice() : local);

// From verdaccio@5.1.6/packages/middleware/src/middleware.ts
errorReportingMiddleware(req, res, _.noop);

// From <https://github.com/odoo/odoo/tree/15.0/addons/bus/static/src/js/services/bus_service.js>
Promise.resolve(this._audio.play()).catch(_.noop);

// From <https://github.com/ClickHouse/ClickHouse/blob/v21.10.2.15-stable/website/js/docsearch.js>
if (this.$hint.length === 0) {
  this.setHint = this.getHint = this.clearHint = this.clearHintIfInvalid = _.noop;
}

It goes on and on. Many thousands of developers seem to download and use noop helper functions, despite it indeed being literally equivalent to Function.prototype—and probably for the same code-clarity reasons that the proposal explainer extols. I think this can be good enough evidence to at least consider standardization.


fwiw i do like the idea of an API solution for it, even if PFA or the bind operator advances […]

Regarding your footnote, I think it would be on the contrary regarding affecting the chances. From my observation, for a syntax level change (+ competing proposals), you're looking at a good couple years to Stage 3. If Function.uncurryThis [1] gets there first (which is likely), then you can simplify your bind proposal greatly by saying it's basically syntactic sugar for Function.uncurryThis and complain about how verbose Function.uncurryThis is hehe. Then your bind proposal gets acceptance easy-peasy?

It is true that the bind-this operator proposal is looking at several years before it might get standardized and implemented.
(For what it’s worth, this proposal is also looking at several years before it might get fully standardized and implemented.)

However, I am going to be presenting both at the plenary, and I really do not want the representatives there to get confused between them. The operator is not really syntactic sugar for the call-bind function, either: it’s sugar for Function.prototype.bind and Function.prototype.call.

Having said that, the fact that @ljharb (one such Committee representative) is also supportive of a call-bind function, in addition to the bind-this operator, does make me hopeful that the Committee might be amenable to both. I’m just not sure about how it will affect other Committee representatives. If I do add a call-bind–like function to the proposal, I probably wouldn’t do it until this proposal successfully reaches Stage 1.

@bakkot
Copy link

bakkot commented Oct 21, 2021

I am also in favor of uncurryThis, though that might belong on Function.prototype.

I’m hemming and hawing about noop, because apparently _.noop and $.noop truly are used a lot, in a lot of very popular libraries.

I don't think this is very compelling on its own. "People do thing a lot" does not mean thing needs to be blessed by the language - people do a lot of things.

Also, often people (quite reasonably) feel that if a language or library provides a function it's better to use that rather than rolling their own version, even if it would be trivial to do so, so the fact that people are using a noop from a kitchen-sink library they're already using is not strong evidence that they would be put out by having to using ()=>{} instead in a world in which the library did not provide it.

@js-choi
Copy link
Collaborator

js-choi commented Oct 21, 2021

@bakkot: It is true that frequent usage of Thing is not a reason per se to include it in the language proper. Having said that, it is evidence that it may be useful. The basis of this proposal is that:

  1. Thing is [maybe] useful. (The best evidence possible for “Thing is useful” is if “Thing is popular”, although “Thing is popular” is neither necessary nor sufficient for “Thing is useful”.)
  2. Standardization of Thing would help developer ergonomics (not having to define or import your own).
  3. Standardization of Thing would help code clarity (standardizing one name).

Also, often people (quite reasonably) feel that if a language or library provides a function it's better to use that rather than rolling their own version, even if it would be trivial to do so, so the fact that people are using a noop from a kitchen-sink library they're already using is not strong evidence that they would be put out by having to using ()=>{} instead in a world in which the library did not provide it.

Yeah, it’s true that there’s an effect of convenience—whether they would have bothered defining their own function if there wasn’t a library that provided it. But at the same time, plenty of people would also argue that it is better to avoid dependencies on external libraries. It goes both ways. (And plenty of people are importing lodash.noop by itself, rather than the entirety of lodash.)

Anyways, people indeed do a lot of things, but if a lot of people do a thing, then that’s evidence that there’s a cowpath that might deserve to be paved. We should take that as what it is—moderate but not super-strong evidence to standardize—and consider it case by case.


As for a uncurryThis/callThis function, I agree that it would make more sense to put it on Function.prototype than Function. I’m not sure whether that means it should be a separate proposal or part of this proposal…

@bakkot
Copy link

bakkot commented Oct 21, 2021

But at the same time, plenty of people would also argue that it is better to avoid dependencies on external libraries.

My point is that I think what's happening here is that someone says "I need a noop function", and then because they are already using lodash and it provides one they use that, whereas if they were not already using lodash they'd've cheerfully written their own. I agree that many people like to avoid dependencies, but I don't see what bearing that has on this point.

And plenty of people are importing lodash.noop by itself, rather than the entirety of lodash.

... Are they? I see 125 dependents of lodash.noop on npm, and a substantial majority of the ones I sampled are also using a bunch of other stuff from lodash. I would regard those projects as "already using lodash".

@mmkal
Copy link

mmkal commented Oct 21, 2021

IMO a better metric than the number of dependents is weekly downloads. The vast majority of js code isn't open source. 400k/week is a lot.

already using lodash

Most developers don't have any tools that automagically install lodash modular packages. Which means enough human beings painstakingly typed npm install lodash.noop for 400k downloads per week to be happening now. That's a strong signal.

cheerfully writing their own

I have many times uncheerfully written my own as () => {}. Which is uglier and has less clear intent than Function.noop. () => {} is often used as a placeholder function, a kind of // todo: implement, whereas Function.noop doesn't have that ambiguity, in code review etc.

@ljharb
Copy link
Member

ljharb commented Oct 21, 2021

@mmkal 400k/week is quite a small number considering that lodash itself is at ~40M, and other lodash utilities like lodash.throttle have ~3.7 million downloads a week. Additionally, note that download counts include when something is a transitive dependency - so all it would take is one package author (whose package has 400k downloads a week) to type that and publish, for 400k downloads to appear.

@js-choi
Copy link
Collaborator

js-choi commented Oct 22, 2021

My point is that I think what's happening here is that someone says "I need a noop function", and then because they are already using lodash and it provides one they use that, whereas if they were not already using lodash they'd've cheerfully written their own. I agree that many people like to avoid dependencies, but I don't see what bearing that has on this point.

@bakkot: Yeah, I think this is totally plausible. “If it’s not available, I’ll define my own or embed ()=>{}. If it is available, I’ll take it, since it makes my code clearer.” I don’t think this is really an argument one way or the other, though. Just because someone would have cheerfully written their own function if it wasn’t readily available doesn’t mean it’s not worth adding to the language. It just means the stakes are lower either way. The key points are whether a lot of people would use it and whether code clarity would benefit from it.

... Are they? I see 125 dependents of lodash.noop on npm, and a substantial majority of the ones I sampled are also using a bunch of other stuff from lodash. I would regard those projects as "already using lodash".

400k/week is quite a small number considering that lodash itself is at ~40M, and other lodash utilities like lodash.throttle have ~3.7 million downloads a week. Additionally, note that download counts include when something is a transitive dependency - so all it would take is one package author (whose package has 400k downloads a week) to type that and publish, for 400k downloads to appear.

@bakkot, @ljharb: I think these are good points, although my own point with giving download statistics is a general “this is still used a lot, even if it’s not the highest compared to something like throttle”. I guess what I could do, if transitivity is a concern, is quantify occurrences in the Gzemnid dataset, like what we’re doing with the bind-this operator.

For what it’s worth, I consider the inclusion of noop to be a bikeshed before Stage 2. My goal of reaching Stage 1 would be to get Committee consensus that “standardizing at least some Function helpers” is Worth Investigating—whichever those helpers would be.

@zloirock
Copy link

I'm against adding noop. Someone could use it as a constructor placeholder (constructors also are functions) and places that handle this constructor could mutate the prototype or do something else that will change "noop" behavior - if it will be completely immutable, it will break this code, if not - it will break noop.

@bakkot
Copy link

bakkot commented Oct 22, 2021

"If it is available, I’ll take it, since it makes my code clearer."

I think it's even weaker than that. Like I said, many people will reach for the version of a thing provided in the language or in a library they're already using for that reason alone, rather than because it makes the code clearer in their particular case. (Witness people reaching for Function.prototype over ()=>{}, for example - I'd argue that will usually make the code less clear, but people will still reach for Function.prototype just because it's a thing already in the language which has the behavior they want.)

For what it’s worth, I consider the inclusion of noop to be a bikeshed before Stage 2.

Yeah, agreed.

To be clear, I'm not strongly opposed to noop (or identity or constant, for that matter). I am somewhat opposed to all of those, but the main thing I want to convey is that I do not think we should take "lots of people are using the versions of those functions provided in lodash" to be much evidence that they belong in the language.

@js-choi
Copy link
Collaborator

js-choi commented Oct 25, 2021

@bakkot: Yes, it indeed is true that the inclusion of a helper function in a general library like Lodash (or jQuery for that matter, since jQuery also has $.noop) might encourage more usage compared to a separate dependency (like a noop NPM library à la trim-right). But the fact that this is at best weak evidence for standardizing it is also not evidence against standardizing it. It’s a wash either way.

I think that there’s a reasonable argument to be made that noop, is clearer than () => {}. This is of course separate from the fact that at least two general libraries have included it and that it’s popular from those libraries. Its popularity just suggests a cowpath that might deserve to be paved.

But the fact that people who use noop from Lodash and jQuery might not have used noop if the libraries didn’t include them might not be a strong argument for standardization. But neither it is an argument against it either.

Its popularity is only a reason to give it some consideration; further bikeshedding in Stage 2 will have to be on its own merits. We probably all agree about that. 🙂

@celmaun
Copy link

celmaun commented Oct 25, 2021

Me personally, have never needed a noop function. Seems like a code smell or hurting your performance by passing it downstream where god knows how many times it will be called pointlessly. I use null to represent the lack of a function argument (and throw on undefined because it's not explicit/error-prone).

Then you might have some memo function using Function.noop as a WeakMap key where the value can never be GCed. Cross-realm code will also have a hard time detecting a noop.

@celmaun
Copy link

celmaun commented Oct 25, 2021

It's like having an Object.empty = {} to represent the lack of an options argument. Or Array.empty = []... where does it end. null null null 😎

@samhh
Copy link

samhh commented Oct 25, 2021

@Salmatron

This is going a little off course but being able to provide an identity element, such as [] for any given array type, is useful in many functional patterns. It's not always appropriate to represent emptiness as null or its monadic equivalent. Where it's referenced at the call site and not within an abstraction you could just write say [], but something like Array.empty, or better yet a polymorphic mempty better expresses intent.

Back to noop and more traditional JS, the simplest use case I can think of is handling promise rejection where you don't want to do anything on failure. You'll write p().catch(() => {}) and then realise it'd be more readable if you extracted and named the lambda, ending up with p().catch(noop). I've seen patterns like this in lots of codebases; it seems suboptimal to have to redefine something so primitive and widespread in every codebase.

@ljharb
Copy link
Member

ljharb commented Oct 25, 2021

@samhh p().catch() already works - iow, you can pass null or undefined already, and it Just Works, so there's no need for the overhead of creating an empty function.

@zloirock
Copy link

zloirock commented Oct 25, 2021

@samhh p().catch() already works - iow, you can pass null or undefined already, and it Just Works, so there's no need for the overhead of creating an empty function.

Promise.reject().catch() // => Promise<rejected>, Uncaught (in promise) undefined

Promise.reject().catch(() => {}) // => Promise<fulfilled>

@ljharb
Copy link
Member

ljharb commented Oct 25, 2021

Good catch, but that seems like a bug in the HTML spec (and node) to me (the language spec doesn't dictate anything about what logging those rejection notifications produces).

@zloirock
Copy link

zloirock commented Oct 25, 2021

It's not a bug in the HTML spec, it's how ES promises works. The first rejected, the second fulfilled. null or undefined means missed, not empty, callback.

@ljharb
Copy link
Member

ljharb commented Oct 25, 2021

Fair enough.

@js-choi
Copy link
Collaborator

js-choi commented Oct 25, 2021

Me personally, have never needed a noop function. Seems like a code smell or hurting your performance by passing it downstream where god knows how many times it will be called pointlessly. I use null to represent the lack of a function argument (and throw on undefined because it's not explicit/error-prone).

I too have never used noop in any of my code. But, regardless of that, _.noop and $.noop are already used a truly huge number of times in real-world code. This includes high-impact codebases that range from Three.js to Wordpress, and they use them seemingly for various reasons (such as when an API not under your control requires a callback argument). So its standardization is worth at least considering, even if I have not personally used it.

It's like having an Object.empty = {} to represent the lack of an options argument. Or Array.empty = []... where does it end. null null null

I understand this slippery-slope argument, but I don’t think it’s very compelling in this case. No library exports emptyArray or emptyObject variables, and I have found no evidence that any developers prefer emptyArray or emptyObject over [] or {}. In contrast, many people do seem to prefer noop over () => {}, and several libraries do include it. It’s somewhat understandable, since () => {} is wordier and, perhaps, arguably less clear than noop.

Anyways, I personally don’t feel too strongly about noop, though. People could just use constant(), and that’d be equivalent. It’s just another possibility to bikeshed about, later, in any case.

js-choi added a commit that referenced this issue Oct 25, 2021
See #17. This is only tentative.
@celmaun
Copy link

celmaun commented Oct 25, 2021

@js-choi Nicely done! One thing: Is the move to Function.prototype.* deliberate/calculated?

I rather liked keeping all these in Function.* for 2 reasons:

  1. Some of these don't make sense on classes and Object.getPrototypeOf(class Foo {}) === Function.prototype so they'd all essentially become static class methods as well.
  2. Shadowing issues due to people assigning these properties on their functions for a different purpose. Or with static methods like class Foo { static aside() { // I have a different meaning of aside } }

@js-choi
Copy link
Collaborator

js-choi commented Oct 26, 2021

@Salmatron: Well, only one of the old helper functions (aside) has been moved to Function.prototype. Most of the old functions (flow, pipe, constant, identity) remain static methods on Function.

For the prototype-based methods, I was following the precedent set by .bind and .call. I’m not as concerned about them occurring on constructors, although I am concerned about the ergonomics of inline function expressions, like (x => { /*…*/ }).once() versus once(x => { /*…*/ }).

But it’s currently uncertain whether we would even include many of them in this proposal—or, indeed, whether this proposal will reach Stage 1 when I present it to the Committee plenary for Stage 1 tomorrow or on Thursday.

If the proposal successfully reaches Stage 1, we can bikeshed the home of some of these functions more.

@shuckster
Copy link

I like the new suggestions. 👍

@js-choi
Copy link
Collaborator

js-choi commented Oct 28, 2021

I presented this proposal to the Committee at the plenary today. The Committee rejected Stage 1 primarily based on the fact that it’s too broad and should be split up into multiple proposals. I think this is reasonable, and I will do so before presenting again.

  • There was some interest in flow and pipe.
  • There were some objections to standardizing constant, identity, or noop.
  • There were no signals on once or aside.
  • There were strong objections to throttle and debounce, since they depend on timing semantics that are not specified in the language itself; they are defined by the host (HTML/Node/etc.). (Promise.delay was also rejected in the past on these grounds.)
  • There was some interest in unThis.

I will comment here again when I am able to create new proposal repositories. I plan to create repositories for flow/pipe and for unThis first.

@js-choi js-choi closed this as completed Oct 28, 2021
@shuckster
Copy link

I know it wasn't mentioned in the spec, but was there any side-talk on compose?

@celmaun
Copy link

celmaun commented Oct 28, 2021

@js-cho Good luck and thank you for all the great work you're doing to improve the DX for us devs! 💝

@js-choi
Copy link
Collaborator

js-choi commented Oct 28, 2021

I know it wasn't mentioned in the spec, but was there any side-talk on compose?

There was some interest in a LTR flow, but there were no signals on a RTL compose (which I did not present anyway). My impression is that most people would prefer a LTR flow to a RTL compose anyway; see #5.

@js-choi
Copy link
Collaborator

js-choi commented Nov 11, 2021

I’ve split out separate proposals for Function.pipe and flow and unThis. I will archive this proposal sometime after the October plenary notes are published.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

9 participants