Improve output when wrapping functions #15992

liuxingbaoyu · 2023-09-22T10:34:51Z

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

To be honest, after finishing it I found it to be more complicated than I expected.
The main reason is that we need to cache the results of _regeneratorRuntime().mark() to avoid performance regression.
The reason the old code cannot be removed is that we still support bluebird-coroutines.

babel-bot · 2023-09-22T10:36:49Z

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

nicolo-ribaudo · 2023-11-27T16:57:02Z

packages/babel-helpers/src/helpers.ts

+  }
+`;
+
+helpers.asyncToGenerator2 = helper("7.23.0")`


Does this have different behavior than the old asyncToGenerator? If not, we can just replace the old one.

No, I'm just not sure if this affects the compatibility of transform-runtime and helpers, I'll replace it.

nicolo-ribaudo · 2023-11-27T16:58:36Z

packages/babel-helper-remap-async-to-generator/src/index.ts

-            ? callExpression(cloneNode(wrapAwait), [argument.node])
+            ? callExpression(
+                typeof wrapAwait === "string"
+                  ? path.hub.addHelper(wrapAwait)


We have multiple issues with path.hub, maybe we could make wrapAwait be a t.Expression | () => t.Expression and pass it as wrapAwait: () => file.addHelper("...") from the caller?

nicolo-ribaudo · 2023-11-27T16:59:38Z

packages/babel-helper-wrap-function/src/index.ts

+
+function markCallWrapped(path: NodePath<t.Function>) {
+  (path.get("body.body.0.argument") as NodePath).setData(
+    "babel-helper-wrap-function_wrapped_function",


Let's extract this string to a constant, or maybe even just use a weakmap.

packages/babel-helper-wrap-function/src/index.ts

nicolo-ribaudo · 2023-11-27T17:02:38Z

packages/babel-plugin-proposal-function-sent/src/index.ts

@@ -47,6 +47,7 @@ export default declare(api => {
    inherits: syntaxFunctionSent,

    visitor: {
+      ...wrapFunction.buildOnCallExpression("callSkipFirstGeneratorNext"),


We should use traverse.visitor.merge, in case we will add visitors that cover the same nodes.

nicolo-ribaudo · 2023-11-27T17:10:56Z

packages/babel-helper-wrap-function/src/index.ts

+export function buildOnCallExpression(helperName: string) {
+  return {
+    CallExpression: {
+      exit(path: NodePath<t.CallExpression>, state: PluginPass) {


Why do we need to do this on exit, and not when we call wrapFunction?

https://github.com/babel/babel/pull/15992/files#diff-84a64030b885f05d8fef6726ef5a9c863a2761756c846ff5ca9278a058384151L1-L11
In the past, we always created a closure for the function to save, which was necessary for use cases like babelHelpers.regeneratorRuntime().mark().
Now, since more and more users support generators natively, I chose not to create closures for all functions, but only save the results for use cases like regeneratorRuntime.
The generator function transformation runs after wrapFunction, and if we modify that plugin directly, we will have to deal with compatibility issues.
To be honest, after I finished it, I thought it was more complicated than I expected, but it's done and there's no harm in it.

packages/babel-helper-wrap-function/src/index.ts

liuxingbaoyu · 2023-12-02T04:55:35Z

...ransform-async-to-generator/test/fixtures/async-to-generator/async-arrow-in-method/output.js

  name: "John Doe",
  testMethodFailure() {
    var _this = this;
-    return new Promise( /*#__PURE__*/function () {
-      var _ref = babelHelpers.asyncToGenerator(function* (resolve) {
+    return new Promise(function (_x) {
+      return babelHelpers.callAsync(function* (resolve) {
        console.log(_this);
        setTimeout(resolve, 1000);
-      });
-      return function (_x) {
-        return _ref.apply(this, arguments);
-      };
-    }());
+      }, this, arguments);
+    });
  }
 };


If we don't use OnCallExpression then we will have to generate output like this.

let TestClass = { name: "John Doe", testMethodFailure() { var _this = this; return new Promise(function (_x) { return (ref = ref || babelHelpers.asyncToGenerator(function* (resolve) { console.log(_this); setTimeout(resolve, 1000); })).apply(this, arguments); }); } };

liuxingbaoyu · 2023-12-02T05:04:01Z

packages/babel-helpers/src/helpers.ts

@@ -34,38 +34,45 @@ helpers.wrapAsyncGenerator = helper("7.0.0-beta.0")`
  }
 `;

-helpers.asyncToGenerator = helper("7.0.0-beta.0")`


I'm not sure if the version should be modified.

I will add new-version-checklis after #15959 is merged.

@liuxingbaoyu Just a kind reminder that #15959 has been merged.

Co-authored-by: Jan Jones <jan.jones.cz@gmail.com>

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>

jridgewell · 2024-03-25T17:23:45Z

packages/babel-helpers/src/helpers.ts

+    return new Promise(function (resolve, reject) {
+      function step(key, arg) {
+        try {
+          var info = gen[key](arg);


While we're here, we can gain ~30% performance improvement by changing key into a boolean, and changing this to happy ? gen.next(arg) : gen.throw(arg)

jridgewell · 2024-03-25T17:25:21Z

...proposal-explicit-resource-management/test/fixtures/integration/async-to-generator/output.js

-}
-function _fn() {
-  _fn = babelHelpers.asyncToGenerator(function* () {
+  return babelHelpers.callAsync(function* () {


Was the intention of the old code to keep this as a top-level function, so we don't need to reevaluate it constantly? I don't think it achieved that, but it would help overall performance.

Initially it was a variable declaration + a function declaration, but later it became two function declarations to fix the hoisting problem.

Perhaps initially creating a new closure might be to facilitate working with bluebird/regenerator.
Because they're better off calling regenerator.mark/bluebird.coroutine only once on the function body.
(I'm not sure about this, just guessing)

The original function declaration was probably to preserve fn.length and fn.name.

liuxingbaoyu added the PR: Output optimization 🔬 A type of pull request used for our changelog categories label Sep 22, 2023

liuxingbaoyu force-pushed the wrapper branch 3 times, most recently from 39776f4 to 61b2cee Compare September 22, 2023 23:45

liuxingbaoyu force-pushed the wrapper branch from 7da248f to bf5b052 Compare November 19, 2023 06:37

nicolo-ribaudo reviewed Nov 27, 2023

View reviewed changes

jjonescz reviewed Nov 29, 2023

View reviewed changes

packages/babel-helper-wrap-function/src/index.ts Outdated Show resolved Hide resolved

liuxingbaoyu force-pushed the wrapper branch from 1719bd2 to 469755a Compare December 2, 2023 04:44

liuxingbaoyu commented Dec 2, 2023

View reviewed changes

liuxingbaoyu and others added 9 commits March 25, 2024 14:35

improve

4778e6a

fix

cbdd7e6

fix

4a78642

improve

925294f

improve

083b4ab

Update packages/babel-helper-wrap-function/src/index.ts

b05cedb

Co-authored-by: Jan Jones <jan.jones.cz@gmail.com>

Update packages/babel-helper-wrap-function/src/index.ts

6abad87

Co-authored-by: Nicolò Ribaudo <nicolo.ribaudo@gmail.com>

review

f208c4c

rebase

a74268d

liuxingbaoyu force-pushed the wrapper branch from 469755a to a74268d Compare March 25, 2024 09:51

jridgewell reviewed Mar 25, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve output when wrapping functions #15992

Improve output when wrapping functions #15992

liuxingbaoyu commented Sep 22, 2023 •

edited by gitpod-io bot

babel-bot commented Sep 22, 2023 •

edited

nicolo-ribaudo Nov 27, 2023

liuxingbaoyu Dec 2, 2023

nicolo-ribaudo Nov 27, 2023

nicolo-ribaudo Nov 27, 2023

nicolo-ribaudo Nov 27, 2023

nicolo-ribaudo Nov 27, 2023

liuxingbaoyu Nov 29, 2023

liuxingbaoyu Dec 2, 2023 •

edited

liuxingbaoyu Dec 2, 2023

nyngwang Mar 22, 2024

jridgewell Mar 25, 2024 •

edited

jridgewell Mar 25, 2024

liuxingbaoyu Mar 25, 2024 •

edited

Improve output when wrapping functions #15992

Are you sure you want to change the base?

Improve output when wrapping functions #15992

Conversation

liuxingbaoyu commented Sep 22, 2023 • edited by gitpod-io bot

babel-bot commented Sep 22, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liuxingbaoyu Dec 2, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jridgewell Mar 25, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liuxingbaoyu Mar 25, 2024 • edited

Choose a reason for hiding this comment

liuxingbaoyu commented Sep 22, 2023 •

edited by gitpod-io bot

babel-bot commented Sep 22, 2023 •

edited

liuxingbaoyu Dec 2, 2023 •

edited

jridgewell Mar 25, 2024 •

edited

liuxingbaoyu Mar 25, 2024 •

edited