Improve output of `using` #15959

liuxingbaoyu · 2023-09-12T09:29:05Z

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

To be honest I think this may not be an improvement over bundled + minimized size, but it should make the output easier to view.

babel-bot · 2023-09-12T09:31:27Z

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

nicolo-ribaudo · 2023-09-12T10:03:33Z

packages/babel-helpers/src/helpers/usingCtx.js

+  return {
+    s: [], // stack
+    e: empty, // error
+    using: function (value, isAwait) {


I'm not sure that this PR is worth doing, but anyway we can improve this helper:

use u and d rather than using and dispose

s can be a local variable, rather than a property

you might split u in u and a ("using" and "await using"), to save on the second parameter

liuxingbaoyu · 2023-09-12T10:43:24Z

CI errors are funny. I will investigate.

nicolo-ribaudo · 2023-09-12T10:53:27Z

We do not support having two helpers with two top-level variables having the same name 😁

nicolo-ribaudo · 2023-09-15T10:09:18Z

packages/babel-helpers/src/helpers/usingCtx.js

@@ -0,0 +1,78 @@
+/* @minVersion 7.23.0 */
+
+export default function _usingCtxFactory() {


Could you wrap the old helper rather than the new one, so that the new one remains smaller?

nicolo-ribaudo · 2023-09-15T10:10:39Z

packages/babel-helpers/src/helpers/usingCtx.js

+        }
+
+        function err(e) {
+          error = error != empty ? new dispose_SuppressedError(e, error) : e;


Suggested change

error = error != empty ? new dispose_SuppressedError(e, error) : e;

error = error !== empty ? new dispose_SuppressedError(e, error) : e;

If something does throw "[object Object]", error == empty will be true :P

nicolo-ribaudo · 2023-09-15T10:17:52Z

packages/babel-helpers/src/helpers/usingCtx.js

+          while ((r = stack.pop())) {
+            try {
+              var r,
+                p = r.d.call(r.v);


The var r here is incredibly confusing, I was initially surprised that it's not undefined :P Unfortunately Terser cannot do this optimization for us so it's fine to do it manually, but I opened terser/terser#1446.

Also, let's rename r to resource and p to disposalResult since we now mangle variables anyway 😉

JLHwung · 2023-11-17T14:59:14Z

packages/babel-plugin-proposal-explicit-resource-management/src/index.ts

+        t.addComment(
+          replacement.block.body[0],
+          "leading",
+          "u: using(obj, isAwait), d: dispose()",


We generally do not add comments explaining the transpiled output. Though we could do so under a verbose: boolean flag.

I will remove them and open a separate PR if needed in the future.

JLHwung

Can you add a todo item in the make new-version-checklist?
Since we have to update the minVersion data of the usingCtx helper.

liuxingbaoyu · 2023-11-29T06:00:29Z

packages/babel-helpers/src/helpers/usingCtx.js

+  var empty = {},
+    stack = [];
+  function using(isAwait, value) {
+    if (value != null) {


I used v!=null here because it doesn't look like people would use document.all here.
And https://github.com/babel/babel/pull/15959/files#diff-cd9817b5f60ed5ac93a50bd1667f5f8216d36a6d1eb52ade7ad3ebe6bcaaf21cR34

liuxingbaoyu · 2023-11-29T07:11:34Z

packages/babel-helpers/src/helpers/usingCtx.ts

+  var _disposeSuppressedError =
+      typeof SuppressedError === "function"
+        ? // eslint-disable-next-line no-undef
+          SuppressedError
+        : (function (suppressed: boolean, error: Error) {
+            var err = new Error() as SuppressedError;
+            err.name = "SuppressedError";
+            err.suppressed = suppressed;
+            err.error = error;
+            return err;
+          } as SuppressedErrorConstructor),


I borrowed from ts's implementation because theirs is much simpler, I think the behavior has some differences in the prototype, but I'm not sure who is right and if it is acceptable.

Also sorry for making these changes after the review. :) @JLHwung

ts helper

var __disposeResources = (this && this.__disposeResources) || (function (SuppressedError) { return function (env) { function fail(e) { env.error = env.hasError ? new SuppressedError(e, env.error, "An error was suppressed during disposal.") : e; env.hasError = true; } function next() { while (env.stack.length) { var rec = env.stack.pop(); try { var result = rec.dispose && rec.dispose.call(rec.value); if (rec.async) return Promise.resolve(result).then(next, function (e) { fail(e); return next(); }); } catch (e) { fail(e); } } if (env.hasError) throw env.error; } return next(); }; })(typeof SuppressedError === "function" ? SuppressedError : function (error, suppressed, message) { var e = new Error(message); return e.name = "SuppressedError", e.error = error, e.suppressed = suppressed, e; });

https://github.com/es-shims/SuppressedError/blob/11f592cc4ec88691b20944057b1772e4b62d201c/implementation.js#L13

By looking at it, I think the previous implementation was more spec-compliant, but it seems like ts's behavior is sufficient for use here.
Since we did not assign a value to SuppressedError, users should not use instanceof to compare it.

liuxingbaoyu · 2023-12-01T07:43:16Z

packages/babel-helpers/src/helpers/usingCtx.ts

+      typeof SuppressedError === "function"
+        ? // eslint-disable-next-line no-undef
+          SuppressedError
+        : (function (suppressed: boolean, error: Error) {


Oops, I realized that our two parameters were reversed, and it is not boolean. : :)

liuxingbaoyu · 2024-01-09T11:51:01Z

Since rebasing made this PR more complicated, I removed the new-version-checklist changes and I will update the version of this PR synchronously after the new patch is released.

liuxingbaoyu · 2024-01-12T03:45:47Z

packages/babel-helpers/src/helpers/usingCtx.ts

+          "using declarations can only be used with objects, functions, null, or undefined.",
+        );
+      }
+      // core-js-pure uses Symbol.for for polyfilling well-known symbols


Do we expect users to use the core-js-pure conversion helper? 🤔

Not necessarily, but maybe users are importing "things that can be disposed" from libraries that have been compiled with core-js, and that thus use Symbol.for("Symbol.dispose") as their protocol.

nicolo-ribaudo

Great! As a follow-up PR, please remove the old transform for Babel 8.

This PR contains the following updates: | Package | Type | Update | Change | |---|---|---|---| | [@babel/traverse](https://babel.dev/docs/en/next/babel-traverse) ([source](https://github.com/babel/babel)) | resolutions | patch | [`7.23.7` -> `7.23.9`](https://renovatebot.com/diffs/npm/@babel%2ftraverse/7.23.7/7.23.9) | --- ### Release Notes <details> <summary>babel/babel (@babel/traverse)</summary> ### [`v7.23.9`](https://github.com/babel/babel/blob/HEAD/CHANGELOG.md#v7239-2024-01-25) [Compare Source](babel/babel@v7.23.7...v7.23.9) ##### 🐛 Bug Fix - `babel-helper-transform-fixture-test-runner`, `babel-plugin-transform-function-name`, `babel-plugin-transform-modules-systemjs`, `babel-preset-env` - [#16225](babel/babel#16225) fix: `systemjs` re-traverses helpers ([@liuxingbaoyu](https://github.com/liuxingbaoyu)) - `babel-helper-create-class-features-plugin`, `babel-plugin-proposal-decorators` - [#16226](babel/babel#16226) Improve decorated private method check ([@JLHwung](https://github.com/JLHwung)) - `babel-plugin-proposal-decorators`, `babel-plugin-transform-async-generator-functions`, `babel-plugin-transform-runtime`, `babel-preset-env` - [#16224](babel/babel#16224) Properly sort `core-js@3` imports ([@nicolo-ribaudo](https://github.com/nicolo-ribaudo)) - `babel-traverse` - [#15383](babel/babel#15383) fix: Don't throw in `getTypeAnnotation` when using TS+inference ([@liuxingbaoyu](https://github.com/liuxingbaoyu)) - Other - [#16210](babel/babel#16210) \[eslint] Fix `no-use-before-define` for class ref in fields ([@nicolo-ribaudo](https://github.com/nicolo-ribaudo)) ##### 🏠 Internal - `babel-core`, `babel-parser`, `babel-template` - [#16222](babel/babel#16222) Migrate `eslint-parser` to cts ([@liuxingbaoyu](https://github.com/liuxingbaoyu)) - `babel-types` - [#16213](babel/babel#16213) Remove `@babel/types` props that are not produced by the parser ([@liuxingbaoyu](https://github.com/liuxingbaoyu)) ##### 🏃‍♀️ Performance - `babel-parser` - [#16072](babel/babel#16072) perf: Improve parser performance for typescript ([@liuxingbaoyu](https://github.com/liuxingbaoyu)) ##### 🔬 Output optimization - `babel-helper-create-class-features-plugin`, `babel-plugin-proposal-decorators`, `babel-plugin-proposal-destructuring-private`, `babel-plugin-proposal-pipeline-operator`, `babel-plugin-transform-class-properties`, `babel-plugin-transform-class-static-block`, `babel-plugin-transform-new-target`, `babel-plugin-transform-parameters`, `babel-plugin-transform-private-methods`, `babel-preset-env` - [#16218](babel/babel#16218) Improve temporary variables for decorators ([@liuxingbaoyu](https://github.com/liuxingbaoyu)) - `babel-helpers`, `babel-plugin-proposal-explicit-resource-management`, `babel-runtime-corejs2`, `babel-runtime-corejs3`, `babel-runtime` - [#15959](babel/babel#15959) Improve output of `using` ([@liuxingbaoyu](https://github.com/liuxingbaoyu)) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).  Reviewed-on: https://gitea.vylpes.xyz/RabbitLabs/random-bunny/pulls/142 Reviewed-by: Vylpes <ethan@vylpes.com> Co-authored-by: Renovate Bot <renovate@vylpes.com> Co-committed-by: Renovate Bot <renovate@vylpes.com>

liuxingbaoyu force-pushed the improve-using branch from 0e10e0a to 376555f Compare September 12, 2023 09:32

nicolo-ribaudo reviewed Sep 12, 2023

View reviewed changes

nicolo-ribaudo reviewed Sep 15, 2023

View reviewed changes

liuxingbaoyu force-pushed the improve-using branch from 1f5f6f2 to bb83ab5 Compare October 9, 2023 20:56

liuxingbaoyu force-pushed the improve-using branch from bb83ab5 to 8ba69ba Compare November 17, 2023 13:30

JLHwung reviewed Nov 17, 2023

View reviewed changes

liuxingbaoyu force-pushed the improve-using branch from 8ba69ba to d72e27c Compare November 18, 2023 06:23

liuxingbaoyu added Spec: Explicit Resource Management PR: Output optimization 🔬 A type of pull request used for our changelog categories labels Nov 19, 2023

JLHwung approved these changes Nov 27, 2023

View reviewed changes

liuxingbaoyu force-pushed the improve-using branch 2 times, most recently from f8a0483 to 0defb46 Compare November 29, 2023 05:59

liuxingbaoyu commented Nov 29, 2023

View reviewed changes

liuxingbaoyu force-pushed the improve-using branch from 0defb46 to b512c42 Compare November 29, 2023 07:07

liuxingbaoyu commented Nov 29, 2023

View reviewed changes

liuxingbaoyu commented Dec 1, 2023

View reviewed changes

liuxingbaoyu force-pushed the improve-using branch 3 times, most recently from a291a24 to d0ec5ed Compare December 1, 2023 14:56

JLHwung self-requested a review December 1, 2023 16:29

liuxingbaoyu mentioned this pull request Dec 2, 2023

Improve output when wrapping functions #15992

Open

liuxingbaoyu force-pushed the improve-using branch from d0ec5ed to b1187c7 Compare December 13, 2023 07:25

liuxingbaoyu requested a review from nicolo-ribaudo December 18, 2023 11:43

liuxingbaoyu force-pushed the improve-using branch from f98d8d3 to 334da77 Compare January 9, 2024 11:48

liuxingbaoyu commented Jan 12, 2024

View reviewed changes

nicolo-ribaudo approved these changes Jan 15, 2024

View reviewed changes

liuxingbaoyu and others added 10 commits January 15, 2024 13:36

improve

ff23d28

fix

8df7565

improve

f9e8ff3

review

bd20e3a

improve

ebb7847

improve from ts

5e7ba63

fix

ae14da3

rebase

f79d39c

rebase

3a5ca3f

Update makefile

5d8432f

nicolo-ribaudo force-pushed the improve-using branch from 334da77 to 5d8432f Compare January 15, 2024 12:41

nicolo-ribaudo merged commit 218faee into babel:main Jan 15, 2024
47 checks passed

github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label Apr 16, 2024

github-actions bot locked as resolved and limited conversation to collaborators Apr 16, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve output of `using` #15959

Improve output of `using` #15959

liuxingbaoyu commented Sep 12, 2023 •

edited by gitpod-io bot

babel-bot commented Sep 12, 2023 •

edited

nicolo-ribaudo Sep 12, 2023

liuxingbaoyu commented Sep 12, 2023

nicolo-ribaudo commented Sep 12, 2023

nicolo-ribaudo Sep 15, 2023

nicolo-ribaudo Sep 15, 2023

nicolo-ribaudo Sep 15, 2023 •

edited

JLHwung Nov 17, 2023

liuxingbaoyu Nov 18, 2023

JLHwung left a comment

liuxingbaoyu Nov 29, 2023 •

edited

liuxingbaoyu Nov 29, 2023

liuxingbaoyu Dec 13, 2023

liuxingbaoyu Dec 1, 2023 •

edited

liuxingbaoyu commented Jan 9, 2024

liuxingbaoyu Jan 12, 2024 •

edited

nicolo-ribaudo Jan 15, 2024

nicolo-ribaudo left a comment

		@@ -0,0 +1,78 @@
		/* @minVersion 7.23.0 */

		export default function _usingCtxFactory() {

	error = error != empty ? new dispose_SuppressedError(e, error) : e;
	error = error !== empty ? new dispose_SuppressedError(e, error) : e;

Improve output of using #15959

Improve output of using #15959

Conversation

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

babel-bot commented Sep 12, 2023 • edited

Choose a reason for hiding this comment

liuxingbaoyu commented Sep 12, 2023

nicolo-ribaudo commented Sep 12, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolo-ribaudo Sep 15, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JLHwung left a comment

Choose a reason for hiding this comment

liuxingbaoyu Nov 29, 2023 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

liuxingbaoyu Dec 1, 2023 • edited

Choose a reason for hiding this comment

liuxingbaoyu commented Jan 9, 2024

liuxingbaoyu Jan 12, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nicolo-ribaudo left a comment

Choose a reason for hiding this comment

Improve output of `using` #15959

Improve output of `using` #15959

liuxingbaoyu commented Sep 12, 2023 •

edited by gitpod-io bot

babel-bot commented Sep 12, 2023 •

edited

nicolo-ribaudo Sep 15, 2023 •

edited

liuxingbaoyu Nov 29, 2023 •

edited

liuxingbaoyu Dec 1, 2023 •

edited

liuxingbaoyu Jan 12, 2024 •

edited