Partial application plugin by byara · Pull Request #9474 · babel/babel

byara · 2019-02-08T15:27:51Z

Q	A
Fixed Issues?	Fixes #9342
Patch: Bug Fix?	No
Major: Breaking Change?	No
Minor: New Feature?	Yes
Tests Added + Pass?	Yes
Documentation PR Link
Any Dependency Changes?
License	MIT

This PR addresses the proposal regarding Partial Application plugin.

The syntax rules that was suggested in the proposal to be covered:

- Given f(?), the expression f is evaluated immediately.
- Given f(?, x), the non-placeholder argument x is evaluated immediately and fixed in its position.
- Given f(?), excess arguments supplied to the partially applied function result are ignored.
- Given f(?, ?) the partially applied function result will have a parameter for each placeholder token that is supplied in that token's position in the argument list.
- Given f(this, ?), the this in the argument list is the lexical this.
- Given f(?), the this receiver of the function f is fixed as undefined in the partially applied function result.
- Given f(?), the length of the partially applied function result is equal to the number of ? placeholder tokens in the argument list.
- Given f(?), the name of the partially applied function result is f.name.
- Given o.f(?), the references to o and o.f are evaluated immediately.
- Given o.f(?), the this receiver of the function o.f is fixed as o in the partially applied function result.
- Given f(g(?)), the result is equivalent to f(_0 => g(_0)) not _0 => f(g(_0)). This is because the ? is directly part of the argument list of g and not the argument list of f.

~~I'm not 100% sure about 5, 6, 7 and 8 and how to address them.~~
I welcome any suggestions and reviews to improve this, I'm doing this as part of my thesis and anything that could teach me something is great.

babel-bot · 2019-02-08T15:32:06Z

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/10466/

nicolo-ribaudo · 2019-02-08T19:17:41Z

(I'm marking this as Low priority until the parser PR is merged - don't worry! 😉 )

nicolo-ribaudo · 2019-02-08T19:21:47Z

packages/babel-plugin-proposal-partial-application/src/index.js

+            t.variableDeclaration("const", [
+              t.variableDeclarator(
+                receiverLVal,
+                t.identifier(receiverRVal(node)),


You can use path.scope.push to generate those vars and avoid the iife:

const g = o.f(?, x, 1); // becomes var _receiver, _func, _param, _param2; // generated by scope.push const g = (_receiver = o, _func = o.f, _param = x, _param2 = 1, _arg => _func.call(_receiver, _arg, _param, _param2);

Btw, there is probably some scope/types helper function that says that 1 is a pure constant value and can thus can be transpiled as

const g = (_receiver = o, _func = o.f, _param = x, _arg => _func.call(_receiver, _arg, _param, 1);

I would like to ask for a bit of clarification:

what is the advantage of not using the iife?

could you explain how pushing to scope creates the declarations for me?

danielcaldas

Just a left a few minor comments that might make some parts of the implementation more compact. Great job man! 👍

danielcaldas · 2019-02-08T15:57:26Z

packages/babel-plugin-proposal-partial-application/src/index.js

+  }
+
+  /**
+   * a recursive function that unfolds MemberExpressions within MemberExpression


Maybe a rewording to a recursive function that unfolds nested MemberExpressions

danielcaldas · 2019-02-09T18:55:07Z

packages/babel-plugin-proposal-partial-application/src/index.js

+        return true;
+      }
+    }
+    return false;


Maybe refactor to something like:

function hasArgumentPlaceholder(node) { return node.arguments.some(t.isArgumentPlaceholder) }

is* functions can accept two arguments, so you'll need to wrap it in an arrow to only use one parameter (or use partial application lol)

danielcaldas · 2019-02-09T18:55:59Z

packages/babel-plugin-proposal-partial-application/src/index.js

+  function receiverRVal(node) {
+    let rVal = unfold(node).split(".");
+    rVal.pop();
+    rVal = rVal.join(".");


We can return immediately rVal.join(".");

danielcaldas · 2019-02-09T18:57:35Z

packages/babel-plugin-proposal-partial-application/src/index.js

+   * @returns {Array<Expression>}
+   */
+  function unwrapArguments(node) {
+    const nonPlaceholder = node.arguments.filter(


We can also return immediately here.

danielcaldas · 2019-02-09T18:58:28Z

packages/babel-plugin-proposal-partial-application/src/index.js

+   */
+  function unwrapAllArguments(node, scope) {
+    const clone = t.cloneNode(node);
+    clone.arguments.forEach(argument => {


Maybe refactor into a .map

danielcaldas · 2019-02-09T18:59:34Z

packages/babel-plugin-proposal-partial-application/src/index.js

+   */
+  function argsToVarDeclarator(inits, scope) {
+    let declarator = [];
+    declarator = inits.map(expr =>


We could also return immediately here return inits.map...

danielcaldas · 2019-02-09T19:00:24Z

packages/babel-plugin-proposal-partial-application/src/index.js

+   * @param {Array<Arguments>} args
+   */
+  function mapNonPlaceholderToLVal(nonPlaceholderDecl, allArgsList) {
+    const clone = Array.from(allArgsList);


Array.from(allArgsList) .map(cl => ...);

byara · 2019-02-13T13:54:05Z

@nicolo-ribaudo @danielcaldas thank you for all the reviews, I'll try to address them as soon as possible.

Meanwhile, I have a question:
Regarding this syntactic rule of the proposal:

Given f(?), the name of the partially applied function result is f.name.

What do you think about this solution:

// if we have this:
const foo = bar(1, ?, 3, ?);

// currently converted to this:
const foo = (() => {
  const _func = bar;
  const _param = 1,
        _param2 = 3;
  return (_argPlaceholder, _argPlaceholder2) => _func(_param, _argPlaceholder, _param2, _argPlaceholder2);
})();

bar.name; // => bar
foo.name; // => empty

Instead we do this:

const foo = (() => {
  const _func = bar;
  const _param = 1,
        _param2 = 3;
{
  const bar = (_argPlaceholder, _argPlaceholder2) => _func(_param, _argPlaceholder, _param2, _argPlaceholder2);
  return bar;
}
})();

bar.name; // => bar
foo.name; // => bar

or maybe we can get the foo.name to be foo?

nicolo-ribaudo · 2019-02-13T14:30:41Z

Can we use normal functions?

const add1 = add(1, ?);

// ->

var _func, _param;
const add1 = (_func = add, _param = 1, function add(_placeholder) { return _func(_param, _placeholder) });

byara · 2019-02-13T16:49:48Z

Can we use normal functions?

const add1 = add(1, ?);

// ->

var _func, _param;
const add1 = (_func = add, _param = 1, function add(_placeholder) { return _func(_param, _placeholder) });

It works.
That's a good idea and it looks better. I'll change the plugin accordingly.

nicolo-ribaudo · 2019-02-14T11:18:29Z

packages/babel-plugin-proposal-partial-application/src/index.js

+            t.variableDeclaration("const", [
+              t.variableDeclarator(
+                receiverLVal,
+                t.identifier(receiverRVal(node)),


I don't understand what receiverRVal (which, btw, doesn't generate a valid identifier since the result can contain .) is needed for: isn't using node.callee (and node.callee.property for functionLVal) enough?

Also, it doesn't work:

a.b.fn().c["d"].e(?); // -> (() => { const _receiver = a.b.fn.c.undefined; const _func = a.b.fn.c.undefined.e; return _argPlaceholder => _func.call(_receiver, _argPlaceholder); })();

Oh, and instead of using newFuncLVal/newReceiverLVal you could use path.scope.generateUidIdentifierBasedOnNode(node.callee) and path.scope.generateUidIdentifierBasedOnNode(node.callee.object). It's up to you to decide.

object.get().fn(?); // -> (() => { const _object$get = object.get; const _object$get$fn = object.get.fn; return _argPlaceholder => _object$get$fn.call(_object$get, _argPlaceholder); })();

Oh, and the previous example should be something like this, otuherwise we might execute some accessors twice:

(() => { const _object$get = object.get; const _object$get$fn = _object$get.fn; return _argPlaceholder => _object$get$fn.call(_object$get, _argPlaceholder); })();

nicolo-ribaudo · 2019-02-15T14:52:09Z

.../babel-plugin-proposal-partial-application/test/fixtures/general/member-expression/output.js

+const h = (_p = p, _p$b = p.b, _param2 = y, _param3 = x, function b(_argPlaceholder2) {
+  return _p$b.call(_p, 1, _param2, _param3, 2, _argPlaceholder2);
+});
+const j = (_a$b$c$d$e = a.b.c.d.e, _a$b$c$d$e$foo = a.b.c.d.e.foo, function foo(_argPlaceholder3) {


This should be

const j = (_a$b$c$d$e = a.b.c.d.e, _a$b$c$d$e$foo = _a$b$c$d$e.foo, ...

because a.b.c.d.e could be getters which must be accessed exactly once.

nicolo-ribaudo · 2019-02-15T17:19:20Z

packages/babel-plugin-proposal-partial-application/src/index.js

+   * @param {Array<Node>} args
+   */
+  function mapNonPlaceholderToLVal(nonPlaceholderArgs, allArgsList) {
+    const clonedArgs = Array.from(allArgsList);


Why do you need to clone this array? You are only cloning the array, not its elements.

you are correct, this is useless. I removed it.

nicolo-ribaudo · 2019-02-15T17:19:35Z

packages/babel-plugin-proposal-partial-application/src/index.js

+   */
+  function mapNonPlaceholderToLVal(nonPlaceholderArgs, allArgsList) {
+    const clonedArgs = Array.from(allArgsList);
+    clonedArgs.map(arg => {


Nit: you can use forEach

nicolo-ribaudo · 2019-02-15T17:20:03Z

packages/babel-plugin-proposal-partial-application/src/index.js

+    let cloneList = [];
+    allArgsList.forEach(item => {
+      if (item.name && item.name.includes("_argPlaceholder")) {
+        cloneList = cloneList.concat(item);


cloneList.push(item)

nicolo-ribaudo · 2019-02-15T17:20:48Z

packages/babel-plugin-proposal-partial-application/src/index.js

+          node.callee,
+        );
+        const receiverLVal = path.scope.generateUidIdentifierBasedOnNode(
+          node.callee.object,


This should only be generated when node.callee is a member expression.

nicolo-ribaudo · 2019-02-15T17:28:05Z

Does ...spread work?

foo(a, ...b, ?);

Since the spec doesn't define how it should behave (or I can't find it), we can just throw an error when we find it. Or maybe it uses one of the following semantics? (cc @rbuckton)

var _foo, _a, _b;
_foo = foo, _a = a, _b = b, function foo(_arg) { return _foo(_a, ..._b, _arg) }

// or maybe it calls [Symbol.iterator] earlier

_foo = foo, _a = a, _b = [...b], function foo(_arg) { return _foo(_a, ..._b, _arg) }

rbuckton · 2019-02-15T18:58:51Z

@nicolo-ribaudo: Anything usable in a normal function call should be usable here. All evaluation should be eager, so the second example is correct.

nicolo-ribaudo · 2019-02-15T22:35:16Z

(From the original post)

I'm not 100% sure about 5, 6, 7 and 8 and how to address them.

Don't they already work?

byara · 2019-02-16T09:11:29Z

(From the original post)

I'm not 100% sure about 5, 6, 7 and 8 and how to address them.

Don't they already work?

I added a few tests for 7 and 8. Regarding 5, I think, I sort of understand what "lexical this" means.
For 6, I don't think I understand the "this receiver" for f(?) 😅
For other reviews, I'll address them ASAP.

rbuckton · 2019-02-16T09:36:05Z

Ignoring partial application for a second, when you call f() in strict mode, the receiver (the value used for this inside of f) is undefined. When you call o.f(), the receiver is o. When you do f.call(o), the receiver is o.

6 basically means that when you do g = o.f(?); g(), the value o is preserved as the receiver.

byara · 2019-02-16T11:09:50Z

Ignoring partial application for a second, when you call f() in strict mode, the receiver (the value used for this inside of f) is undefined. When you call o.f(), the receiver is o. When you do f.call(o), the receiver is o.

6 basically means that when you do g = o.f(?); g(), the value o is preserved as the receiver.

@rbuckton Thank you for the explanation. I think 6 is also addressed.

remove unnecessary error message and hasPartial function from parseNewArguments add types for PartialExpression Update the tests rename PartialExpression to Partial move Partial from expressions to types and rename to ArgumentPlaceholder add tests for ArgumentPlaceholder in babel-generator rename Partial to ArgumentPlaceholder update the tests remove alias from the type and undo changes in generated folder adds a nice error message better definition for the type auto-generated files update the conditional for allowPlaceholder message and tests update CallExpression definition to accept ArgumentPlaceholder change description clean up indent ArgumentPlaceholder entry and revert unwanted changes

byara · 2019-02-20T12:51:08Z

@nicolo-ribaudo one of the builds for travis times out. Would you retrigger it?

nicolo-ribaudo · 2019-02-20T13:59:32Z

It seems that after the rebase the plugin disappeared? If you need it, you can restore from https://github.com/nicolo-ribaudo/babel/tree/pr/byara/9474-backup (it is 6 days old so commits after #9474 (review) are not included). If you prefer, I suggest using git reflog to restore the lost commits.

nicolo-ribaudo · 2019-02-21T22:18:18Z

packages/babel-plugin-proposal-partial-application/src/index.js

+      if (t.isArgumentPlaceholder(arg)) {
+        const id = scope.generateUid("_argPlaceholder");
+        placeholders.push(t.identifier(id));
+        args.push(t.cloneNode(t.identifier(id)));


No need to cloneNode here, t.identifier returns a new node.

nicolo-ribaudo · 2019-02-21T22:25:07Z

The failing tests are fixed by #9558

nicolo-ribaudo · 2019-02-21T23:09:22Z

packages/babel-plugin-proposal-partial-application/package.json

+{
+  "name": "@babel/plugin-proposal-partial-application",
+  "version": "7.2.0",
+  "description": "Transform pipeline operator into call expressions",


This needs to be updated to match the readme.

nicolo-ribaudo

🎉

…on-plugin

nicolo-ribaudo · 2019-03-04T23:48:37Z

Could you rebase (or merge master) this PR, since the parser PR has been merged?

…on-plugin

danez

Nice work.

~~Does this work with awaiting async functions? Can we add a test for this?~~ Thinking about it more, I guess that the partial application does not really touch with async functions as await foo(?) is nonsense. await foo(?)(1) could work, but doesn't make much sense. So not sure if a test makes sense.

Also it would be nice to add a test where the result from the call is not assigned but directly chained: foo(?, a)(b).then(()=>{}); Doesn't make much sense either, but I guess it should work.

danez · 2019-03-05T20:09:08Z

.../babel-plugin-proposal-partial-application/test/fixtures/general/multiple-calls/options.json

@@ -0,0 +1,3 @@
+{
+  "plugins": ["proposal-partial-application"]
+}


nit: The options.json files are the same for all tests as I can see, so the config file could be moved on level up into the general folder. Then all the fixtures in general would share the config.

@danez Thanks for the feedback.
I moved options.json one level up and added a new test.

existentialism · 2019-03-13T01:01:08Z

packages/babel-plugin-proposal-partial-application/test/fixtures/general/bound-method/input.js

+  compare(a, b) {
+    if (a > b) {
+      return a;
+    };


Nit, but this semi seems unnecessary?

yep, unnecessary, I'll remove it.

existentialism · 2019-03-13T01:15:11Z

packages/babel-plugin-proposal-partial-application/src/index.js

+              false,
+            ),
+          ]);
+          path.replaceWith(finalExpression);


Not big deal by any means, but since both of these end with replacing the path with a sequenceExpression, we could do:

const sequenceParts = []; if (node.callee.type === "MemberExpression") { sequenceParts.push( /* stuff */ ); } else { sequenceParts.push( /* stuff */ ); } path.replaceWith(t.sequenceExpression(sequenceParts));

good idea! made the necessary changes.

existentialism

Benefit of coming late to the party is the code looks awesome already :)

Nice work!

…on-plugin

nicolo-ribaudo · 2019-03-13T11:20:27Z

Awesome work!

* master: (58 commits) Remove dependency on home-or-tmp package (babel#9678) [proposal-object-rest-spread] fix templateLiteral in extractNormalizedKeys (babel#9628) Partial application plugin (babel#9474) Private Static Class Methods (Stage 3) (babel#9446) gulp-uglify@3.0.2 rollup@1.6.0 eslint@5.15.1 jest@24.5.0 regexpu-core@4.5.4 Remove input and length from state (babel#9646) Switch from rollup-stream to rollup and update deps (babel#9640) System modules - Hoist classes like other variables (babel#9639) fix: Don't transpile ES2018 symbol properties (babel#9650) Add WarningsToErrorsPlugin to webpack to avoid missing build problems on CI (babel#9647) Update regexpu-core dependency (babel#9642) Add placeholders support to @babel/types and @babel/generator (babel#9542) Generate plugins file Make babel-standalone an ESModule and enable flow (babel#9025) Reorganize token types and use a map for them (babel#9645) [TS] Allow context type annotation on getters/setters (babel#9641) ...

nicolo-ribaudo added PR: New Feature 🚀 A type of pull request used for our changelog categories Priority: Low Spec: Partial Application labels Feb 8, 2019

nicolo-ribaudo reviewed Feb 8, 2019

View reviewed changes

danielcaldas reviewed Feb 9, 2019

View reviewed changes

nicolo-ribaudo removed the Priority: Low label Feb 14, 2019

nicolo-ribaudo reviewed Feb 14, 2019

View reviewed changes

nicolo-ribaudo reviewed Feb 15, 2019

View reviewed changes

nicolo-ribaudo added this to the v7.4.0 milestone Feb 18, 2019

byara force-pushed the partial-application-plugin branch 3 times, most recently from e4b72a9 to 9196d67 Compare February 20, 2019 10:50

nicolo-ribaudo mentioned this pull request Feb 20, 2019

Create @babel/plugin-proposal-dynamic-import #9552

Merged

existentialism mentioned this pull request Feb 20, 2019

Add documentation for Partial Application Syntax: Stage 1 babel/website#1974

Closed

4 tasks

byara force-pushed the partial-application-plugin branch from 9196d67 to bb7068a Compare February 20, 2019 16:10

Behrang Yarahmadi added 2 commits February 21, 2019 23:09

update unwrapArguments function and add replacePlaceholders.

ad26310

update the tests

26c2230

nicolo-ribaudo reviewed Feb 21, 2019

View reviewed changes

Behrang Yarahmadi added 3 commits February 21, 2019 23:44

Merge remote-tracking branch 'upstream/master'

ea6d52a

Merge branch 'master' into partial-application-plugin

e3cac87

remove unnecessary clone

45ffe18

nicolo-ribaudo reviewed Feb 21, 2019

View reviewed changes

update readme and description an package.json

2cc2f42

nicolo-ribaudo approved these changes Feb 21, 2019

View reviewed changes

Merge remote-tracking branch 'upstream/master' into partial-applicati…

ac4bb3a

…on-plugin

Merge remote-tracking branch 'upstream/master' into partial-applicati…

fe682e4

…on-plugin

danez reviewed Mar 5, 2019

View reviewed changes

Behrang Yarahmadi added 3 commits March 6, 2019 10:59

put options.json one level up and add test for not assigned and chained

425754d

add await to fix the test

86eab9b

use Promise in the test instead of async await

dabf034

nicolo-ribaudo added the PR: Needs Review A pull request awaiting more approvals label Mar 7, 2019

existentialism reviewed Mar 13, 2019

View reviewed changes

existentialism approved these changes Mar 13, 2019

View reviewed changes

Behrang Yarahmadi added 3 commits March 13, 2019 10:50

remove unnecessary method test [babel#9474]

915b73c

push sequenceExpressions to an array [babel#9474]

e939cf8

Merge remote-tracking branch 'upstream/master' into partial-applicati…

35952e2

…on-plugin

nicolo-ribaudo merged commit 29cd27b into babel:master Mar 13, 2019

rbuckton mentioned this pull request Apr 2, 2019

Transpilation tc39/proposal-partial-application#24

Closed

lock bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Oct 4, 2019

lock bot locked as resolved and limited conversation to collaborators Oct 4, 2019

Uh oh!

Conversation

byara commented Feb 8, 2019 • edited by nicolo-ribaudo Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

babel-bot commented Feb 8, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicolo-ribaudo commented Feb 8, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielcaldas left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

byara commented Feb 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicolo-ribaudo commented Feb 13, 2019

Uh oh!

byara commented Feb 13, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

nicolo-ribaudo Feb 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicolo-ribaudo Feb 14, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

nicolo-ribaudo commented Feb 15, 2019

Uh oh!

rbuckton commented Feb 15, 2019

Uh oh!

nicolo-ribaudo commented Feb 15, 2019

Uh oh!

byara commented Feb 16, 2019

Uh oh!

rbuckton commented Feb 16, 2019

Uh oh!

byara commented Feb 16, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

byara commented Feb 20, 2019

Uh oh!

byara commented Feb 8, 2019 •

edited by nicolo-ribaudo

Loading

babel-bot commented Feb 8, 2019 •

edited

Loading

byara commented Feb 13, 2019 •

edited

Loading

byara commented Feb 13, 2019 •

edited

Loading

nicolo-ribaudo Feb 14, 2019 •

edited

Loading

nicolo-ribaudo Feb 14, 2019 •

edited

Loading

byara commented Feb 16, 2019 •

edited

Loading

nicolo-ribaudo commented Mar 4, 2019 •

edited

Loading

danez left a comment •

edited

Loading

existentialism Mar 13, 2019 •

edited

Loading