TypeScript icon indicating copy to clipboard operation
TypeScript copied to clipboard

A shadowed variable is generated in a an async function with a default parameter declared as a static property of a class when targeting ES5.

Open iclanton opened this issue 1 year ago • 5 comments

🔎 Search Terms

"shadowed", "shadowed variable", "generator"

🕗 Version & Regression Information

  • This changed in commit or PR #56296

⏯ Playground Link

https://www.typescriptlang.org/play?target=1&ts=5.4.3#code/MYGwhgzhAEAa0G8BQ1oAcCuAjEBLY0EALmEftGNALwUQCeAdgQBQQD2AtgKYBqYATgC5oWNmxBcwDatCL8MXAJTUAfIhSpo-LkQz9pYAO5hcROADoszRQG4NAXzsbMOcsVLksMyIxbKqasiaWjp60gBEAOIA8tEAIuF2qI5I9khIACZcoAJc0DlQ6PxswFyFQYQkZARcAB6mzMBsWcIMGBxYXPyKrVwAbl12aUiw5mDWGuZEABZcDI1sDOwS5iBsAOaKk8CkwNPM1qrqwU1L4lzmXcX8zOEAQgCCCbYaqGjFpVCX9UTMAIwvZIvIA

💻 Code

class X {
  public static a = async (someVar: boolean = true) => {
    return await X.b();
  };

  public static b = async () => {
    return "GOOD";
  };
}

declare class process {
  static exit(code: number): never;
}

X.a()
  .then(console.log)
  .catch(() => {
    console.error("BAD");
    process.exit(1);
  });

🙁 Actual behavior

Between TS v5.4.2 and TS v5.3.3, the generated code from the above code sample changes to shadow the _a variable in the outer closure. See diff below:

var X = /** @class */ (function () {
    function X() {
    }
    var _a;
    _a = X;
-    X.a = function (someVar) {
-        if (someVar === void 0) { someVar = true; }
-        return __awaiter(_a, void 0, void 0, function () {
-            return __generator(_a, function (_b) {
-                switch (_b.label) {
+    X.a = function () {
+        var args_1 = [];
+        for (var _i = 0; _i < arguments.length; _i++) {
+            args_1[_i] = arguments[_i];
+        }
+        return __awaiter(_a, __spreadArray([], args_1, true), void 0, function (someVar) {
+            if (someVar === void 0) { someVar = true; }
+            return __generator(_a, function (_a) {
+                switch (_a.label) {
                     case 0: return [4 /*yield*/, _a.b()];
-                    case 1: return [2 /*return*/, _b.sent()];
+                    case 1: return [2 /*return*/, _a.sent()];
                }
            });
        });
    };
    X.b = function () { return __awaiter(_a, void 0, void 0, function () {
        return __generator(_a, function (_b) {
            return [2 /*return*/, "GOOD"];
        });
    }); };
    return X;
}());
X.a()
    .then(console.log)
    .catch(function () {
    console.error("BAD");
    process.exit(1);
});

Note that helpers have been omitted from this code sample.

Notice that in the context of the function, the class/function X is assigned to a variable _a in the "ambient" context of the class. In TS v5.3.3, the async state machine's state variable is declared as _b, but in TS v.5.4.2 is appears as if the context is lost when the state machine is generated, so the state is declared as _a, shadowing the earlier-declared _a.

🙂 Expected behavior

Expected that the async state machine's state variable does not shadow another variable.

Additional information about the issue

No response

This issue was found by @ipip2005.

iclanton avatar Mar 21 '24 22:03 iclanton

@jakebailey, you linked to this issue in https://github.com/microsoft/TypeScript/pull/57898. Is it fixed?

DanielRosenwasser avatar Mar 28 '24 23:03 DanielRosenwasser

Sorry, that was a typo of #57887

jakebailey avatar Mar 29 '24 00:03 jakebailey

@jakebailey - This is still an issue in 5.4.5 (see this playground link). Did the cherry-pick get backed out, or was it not the right fix, or has it not been published yet?

iclanton avatar Apr 22 '24 23:04 iclanton

There is no fix or cherry pick, I typo'd the issue number for a different fix (which is what I was trying to say above but maybe not clearly enough, sorry)

jakebailey avatar Apr 22 '24 23:04 jakebailey

Our organization encountered this during our upgrade to 5.4.4. It was enough to consider reverting back to 5.2, but ultimately we ended up working around it by moving the static functions to be top-level functions instead.

cheit-epic avatar May 02 '24 15:05 cheit-epic