Private property after an optional chain transforms into nonsense code

[jridgewell]

Describe the bug

I think this also broke when we implemented the new transform. Currently, if you have a private property access after an optional chain, the chain will run. The _memo == null conditional check happens before the assignment of _memo.

Input code

class Foo {
  #x;

  test() {
    this?.y.#x
  }
}

Config

{
  "jsc": {
    "parser": {
      "syntax": "ecmascript",
      "jsx": false
    },
    "target": "es2021",
    "loose": false,
    "minify": {
      "compress": false,
      "mangle": false
    }
  },
  "module": {
    "type": "commonjs"
  },
  "minify": false,
  "isModule": false
}

Playground link

https://play.swc.rs/?version=1.3.66&code=H4sIAAAAAAAAA0vOSSwuVnDLz1eo5lJQUK6w5gJSJanFJRqaYBEgJyOz2F6vUk%2B5Asit5aoFAHwm0I8yAAAA&config=H4sIAAAAAAAAA1VPOw7DIAzdcwrkuUObMXfoISzqRET8hKlUFOXuBQJps%2Fn95W0QAlaWMIktnxl4DEzhxJnhZCN%2BMgMkDbIMyke4dXXlIs2omSq1HwpEDAvFmuLxPj5aArRzTD3ROKOsmtP%2FpnTGB2K%2BGosV7aLpuji0VTDu9a5i%2ByUmX1BpM86uDD9nXzzbQfGzx4%2Fu%2FQutaQ0NHQEAAA%3D%3D

Expected behavior

The memo is assigned before the conditional, and the private access is scoped to the conditional's alternate branch:

// …
var _x = new WeakMap();
class Foo {
    test() {
        (_this = this) === null || _this === void 0
          ? void 0
          : _class_private_field_get(_this.y , _x);
    }
    // …
}

Actual behavior

The chain is nonsense:

// …
var _x = new WeakMap();
class Foo {
    test() {
        var _this_y, _this_y1;
        _this_y === null || _this_y === void 0
            ? void 0
            : _class_private_field_get(_this_y1 = this?.y, _x);
    }
    // …
}

Version

1.3.66

Additional context

No response

[komyg]

Hi, can I fix this bug?

Also, I am getting this error: An optional chain cannot contain private identifiers.(18030) on the Typescript playground.

So maybe this isn't supported yet?

I did a quick Google search and found this issue, but I didn't understand if optional chaining of private identifiers is not yet approved by TC-39.

[kdy1]

@komyg Of course, thank you!

Babel supports this. https://babeljs.io/repl#?browsers=chrome%2085&build=&builtIns=false&corejs=3.21&spec=false&loose=false&code_lz=MYGwhgzhAEBiD29oG8BQ1oGIAeBuV60ALgKYREAUAlCoRkQBYCWEA_AHQCe7OhAvqj5A&debug=false&forceAllTransforms=false&modules=false&shippedProposals=false&circleciRepo=&evaluate=false&fileSize=false&timeTravel=false&sourceType=module&lineWrap=true&presets=env%2Creact%2Cstage-2&prettier=false&targets=&version=7.22.5&externalPlugins=&assumptions=%7B%7D

[jridgewell]

@komyg: If you're new to SWC, it'll probably be easier to fix the related #7559.

Also, I am getting this error: An optional chain cannot contain private identifiers.(18030) on the Typescript playground.

That's valid syntax (I'm a co-champion of the Optional Chaining proposal in TC39), but TS never added support. Private field usage is still pretty low in my experience, so maybe not enough people are hitting this or working around it in other ways.

[komyg]

@komyg: If you're new to SWC, it'll probably be easier to fix the related #7559.

Of course, could you assign it to me as well?

[kdy1]

@komyg Can you left a comment there?

[komyg]

Hey @jridgewell,

The issue seems to be here: https://github.com/swc-project/swc/blob/cf902d38083ff899eb7a141d5fb68f886f0b9f81/crates/swc_ecma_transforms_compat/src/es2020/optional_chaining.rs#L247C23-L247C23.

From what I've seen, we just treat the member variable as an expression, so we don't take into account that it can be a private member.

To solve this, I think we should do something like:

Gathering::Member(mut m) => {
    m.obj = Box::new(current);
    ctx = None;

    if (m.prop.is_private_name()) {
      // Do some transform here.
    }

Does this makes sense?

I saw that we have methods like: visit_mut_private_get, that I think would handle the private variable correctly and return the appropriate: _class_private_field_get call. But I haven't figured it out how to import and call it. Could you help me?

[kdy1]

@komyg It makes sense, but you should not call it directly, and

class Foo {
  #x;

  test() {
    this?.y.#x;
  }
}

should be compiled as

class Foo {
  #x;
  test() {
    this === null || this === void 0 ? void 0 : this.y.#x;
  }
}

by the optional chaining transform

[kdy1]

Reopening as #7812 is reverted

[swc-bot]

This closed issue has been automatically locked because it had no new activity for a month. If you are running into a similar issue, please create a new issue with the steps to reproduce. Thank you.