Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Disable define/set issue transform for targets with native class properties support #9743

Open
eps1lon opened this issue Nov 18, 2024 · 2 comments · Fixed by #9746
Open

Disable define/set issue transform for targets with native class properties support #9743

eps1lon opened this issue Nov 18, 2024 · 2 comments · Fixed by #9746
Assignees
Labels
Milestone

Comments

@eps1lon
Copy link

eps1lon commented Nov 18, 2024

Describe the bug

When env.targets targets an environment with support for class properties, SWC will still transpile in a constructor implementation.

This is problematic since throwing an error from the parent class constructor would create a stackframe that can't be fully sourcemapped since the super(...args) call in the generated code does not appear in the original code.

Alternatively, #9742 would fix the main motivation for this ticket.

Input code

class Foo extends Bar {
  handleScroll = () => {}
}

Config

{
  "jsc": {
    "parser": {
      "syntax": "ecmascript",
      "jsx": false
    },
    "loose": true,
    "minify": {
      "compress": false,
      "mangle": false
    }
  },
  "module": {
    "type": "es6"
  },
  "minify": false,
  "isModule": true,
  "env": {
    "targets": "node >= 18.18"
  }
}

Playground link (or link to the minimal reproduction)

https://play.swc.rs/?version=1.9.2&code=H4sIAAAAAAAAA0vOSSwuVnDLz1dIrShJzUspVnBKLFKo5lJQyEjMS8lJDU4uys%2FJUbBV0NBUsLVTqK7lqgUAn5sc8jMAAAA%3D&config=H4sIAAAAAAAAA1WOTQrDIBBG955CZl0K2ZRQaG%2FQQ4iZBEP8wTGlEnL3GBttupv5nvP8FsY5jCThzpc0psUJT%2BjrnhKKJohPSgClFiS9cgEuhY60o15MhDlavwQmawkTCX7GI9LKqD6e1dJq55GoGKpVCzNM%2BC9mhxy07eYMj8ohOsz16Aa%2FR%2BWzKgZFr3JZWgGa90kk%2FIBhbwPGdsifD96016bNVrZud4rKzC0BAAA%3D

SWC Info output

No response

Expected behavior

Input code is not modified.

Actual behavior

Transpiled code contains super(...args) call that can't be sourcemapped to original code due to #9742.

Version

1.9.2

Additional context

No response

@eps1lon eps1lon added the C-bug label Nov 18, 2024
@kdy1 kdy1 self-assigned this Nov 19, 2024
@kdy1 kdy1 added this to the Planned milestone Nov 19, 2024
@kdy1
Copy link
Member

kdy1 commented Nov 19, 2024

There are two bugs here, one for documentation and one for the default value of the Assumptions API.

https://play.swc.rs/?version=1.9.2&code=H4sIAAAAAAAAA0vOSSwuVnDLz1dIrShJzUspVnBKLFKo5lJQyEjMS8lJDU4uys%2FJUbBV0NBUsLVTqK7lqgUAn5sc8jMAAAA%3D&config=H4sIAAAAAAAAA22PzQrCMBCE7z5F2HMRepEi6EXwJvgKMd2WlPyRTaWl9N1NY1sregnJzOy3k2HHGDQk4MiGeI0PTtRqF6Q1tIpRJgz39qGkuKiYuEpU5eRXXBGmzJi95x33hP5rtDeBd1EBFJqT8NIFyBa3oe4vR1lLGJ3gW5wlLY2s%2Bi1aWO080tpkpWpuaoW%2F4HSAtmWb3PnPoXeY%2BtEBPqFl20oGSbdlcqkFaJ4bEPc1hqkOGFsiO59YXuzzIlF34wseLtIzbgEAAA%3D%3D

{
  "jsc": {
    "assumptions": {
      "setPublicClassFields": false
    }
}

will disable the problematic behavior. However, assumptions API is not well documented, and @magic-akari Do you think we should adjust the default value of assumptions based on the target.

kdy1 added a commit that referenced this issue Nov 19, 2024
**Related issue:**

 - Closes #9743
@kdy1 kdy1 reopened this Nov 19, 2024
@kdy1 kdy1 closed this as completed Nov 20, 2024
@kdy1 kdy1 reopened this Nov 20, 2024
@kdy1 kdy1 changed the title loose transpiles class properties Adjust default value of assumptions based on the target Nov 20, 2024
@kdy1 kdy1 changed the title Adjust default value of assumptions based on the target Disable define/set issue transform for targets with native class properties support Nov 20, 2024
@magic-akari
Copy link
Member

magic-akari commented Nov 20, 2024

The setPublicClassFields option (or the useDefineForClassFields setting in TypeScript, which controls the same functionality) is disabled by default, aligning with Babel's default configuration.

As for whether assumptions differ based on targets in loose mode, I don't have a clear idea, but we can look to Babel's implementation for guidance.

If we decide to proceed, we'll need to make adjustments in our code. Many of our transformation passes directly pass loose to adjust the output. We should first convert loose to assumptions options.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging a pull request may close this issue.

3 participants