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

Static factory function in decorated class uses un-decorated ctor #4899

Closed
Tracked by #150025
jrieken opened this issue Jun 7, 2022 · 2 comments · Fixed by #4905
Closed
Tracked by #150025

Static factory function in decorated class uses un-decorated ctor #4899

jrieken opened this issue Jun 7, 2022 · 2 comments · Fixed by #4905
Assignees
Labels
Milestone

Comments

@jrieken
Copy link

jrieken commented Jun 7, 2022

Describe the bug

Took me a bit to drill into but in VS Code we have cases where we use a decorator on a class which also has static factory function. Those factory function use the original class constructor, not the decorated one. Checkout the sample, this gist is that calling Foo.create doesn't go through the decorated-code

Input code

function es5ClassCompat(target: Function): any {
	///@ts-expect-error
	function _() { return Reflect.construct(target, arguments, this.constructor); }
	Object.defineProperty(_, 'name', Object.getOwnPropertyDescriptor(target, 'name')!);
	Object.setPrototypeOf(_, target);
	Object.setPrototypeOf(_.prototype, target.prototype);
	return _;
}

@es5ClassCompat
class Foo {

	static create() {
		return new Foo();
	}

    constructor() {}
}

Config

{
  "$schema": "http://json.schemastore.org/swcrc",
  "exclude": "\\.js$",
  "jsc": {
    "parser": {
      "syntax": "typescript",
      "tsx": false,
      "decorators": true
    },
    "target": "es2020",
    "loose": false,
    "minify": {
      "compress": false,
      "mangle": false
    }
  },
  "module": {
    "type": "amd",
    "noInterop": true
  },
  "minify": false
}

Playground link

https://play.swc.rs/?version=1.2.197&code=H4sIAAAAAAAAA32Qy26DMBBF18xXTFcBiYRVN2ETKVW2VP0B5LpDSxRsazwoRRH%2FXlvBpN3UG7%2FOPZYvdKPR0luD5J%2BPF%2BX90Q5OSS6KP0n2eFruiz0qM%2BENsqqqDuK39O1Iy5aYLUO2atq8wBsyycgG36i7BGinrfHCo07aEsM0DmTElyhfvX8QlosaZ8ia93NMflDXG3pl64hlytsSN0YNtClxAYKtuZoEvJDX3LtgWV%2B688VTUa9STxICYmVy1HRReof%2FQXYu7RL8OImx5cNtDTPA4W%2BXoOMaT9aG%2BiDzoqTXqJmUUGwLshQ3dI1YHo3Bg2H8KiayM8w%2FGozC%2B7MBAAA%3D&config=H4sIAAAAAAAAA0WOTQ6DQAhG70JcNmpceoPewc1kxL84wwQw1RjvXqzR7sjj%2Bx7skIkfMDioYVBNdVFMQjG%2FoCgx5sR9IR%2FPHl6Aq5%2BXFi3dNPkkmaFJPNQ7JMeCfE6yRXWrRXRLKJ7HpBZTMdS5WfAFLXpiZ3KBWnnBw9aOe1QroVRlVVphJhJ8KmGMY7edek8hMYr8Vy728508zBWoXU6w%2Fz4wpwutCSO9oyJTeo7e0qv5BTnH4YgMAQAA

Expected behavior

The factory function should invoke the decorated class-contructor. In the SWC output the class and its variable use the same name and I guess JS prefers the class then. In TS the class-variable uses a different name and its static uses refer it by that name.

See how Foo.create calls new Foo_1(); instead of Foo: https://www.typescriptlang.org/play?module=2#code/FAMwrgdgxgLglgewgAgKYGcCsBhANgQ3XWwQFsAHfGAChnwCcBzVGALmQDFJZEIBKdvggBPZAG9gASAD0sgAIx0AWlQAPcqlgr69BPSnho8JMgD61PuOT0WYeigBKqELk0wAdFCToY9MLFoGZhgAGmQgsFJUCEUwmAALOHRPb19-GD0+AG5kAF8pAHkAIwArN3cAE2c4CFQABV0NehhhalMwgHIIfCiOsOKy2HdggoB3CAaEJpaAEQwoejhyDPpAphZO7t6+AEJswtLy9BZJjJaNApA2uKCWfckBo5PdM+ELq9N3cheEc9Qb9Yeb6-X5vVD3GwwOwoUxZYD5YByDA4AhEEgUKjAKCo9CcBAIcTAKQ+KhwKDIBaoKioCyEySSSHQ5C1UZ4hAWOGSBHIHkU1J+WB6WlifK5IA

Actual behavior

The static factory function should use the decoratored constructor calls

Version

1.2.197

Additional context

This is for microsoft/vscode#150025

@jrieken
Copy link
Author

jrieken commented Jul 19, 2022

@kdy1 I am still seeing this error when using the slightly more complex real-case that showed this. This is an updated sample:

https://play.swc.rs/?version=1.2.218&code=H4sIAAAAAAAAA82WS2%2FbMAzHz%2FGn4DCgsQE7PvWSrEu7PoAethRNey4Uh0m02JIhyk2DLt99lB%2BNnT527IAEhs0%2FyR9pSbS3KFRipVaAdHyeCqJzneXC%2BlaYJdohXNX2YAhCbeHZ68VxfGopwqccExuhMdp4vZcwD34Az2DQFkbBLS5SFg0SrciaImnChsCXIkNlKQS7krRXaBOMYOf1JrPfznOOC6nwxugcjd36DyH0lciwH0It4GiTjWoEF0iJkTlHeclU6YMvweglKKFlB6vtNsfJwgWtxB9IBnlz14j3T5xbXfDDyNt5p91WetwpbSwk7hFcyRSnW7KYXbrOAT5ZVHOC6u7Z83pkhZVJKbx8kmTJz5BILHFi7o0cD4EbJdUS%2FsD97TW%2FlsOI%2FIoaHIWbQ3MnWNiycrmPco6mVJ3rOQ72BOFhlJbNVb%2FrUP%2FS9koXav6Z3A3D2%2BSN9U32swtp0C3E7ScXsAd5t4q95HUp1%2FR%2FVNLmeLuQtqJbxy99gyaTRHy0fNo%2B6EC8rqBj7uLfK%2FEoZCpmKX4WfAvhNXrLWIG7g0zMtUq3kLB3gzliQ%2BuE9gsjJ%2BZnxXBQS1g7fsAEJ%2F8gXiu9UXzOurYqwQnHrTlUtoYKPu59TjeQxC3oAAUwhvb9wOppCegzPluHHWvgauvVM6hka6UduNkB4zH0a6h%2BqY5jyIRZ8%2BRCKOcfCIIFlwRU1gR5XVRtJc1SHgSl5wYhEcqd%2B0YkthWjTM8T1D3hzYDy0TWVOA47unxn9E7bfIdfNT5oADfarIVxhxxsVqjqOeMizgqZWqlAl5OOeKzXkoTHlUydxGq4nB6HQIjDKtzK2pyGcbyUdlXMuFVZnMnEaNILG9%2FxEJyWczfayLWMZ6mexZlgThP%2F4AW15pjR%2BUqoJdIgm399YYlKlkgqilK5xupjIhLGiG3EWFEmcv5vI6WjVLO3iVxZTCQX4LvRq%2Fn6jb9LvldjOziY23BycgL95tukX62d3kcedSsPN0pn1pf7pI1QaRKR837FqRXJ%2Bo7fLR5kh6MjqB32S%2BxtQm64khyAXCxwCwXJPX8nUQ29j1pT8m%2F3F2lAVgzhCQAA&config=H4sIAAAAAAAAA0WOTQ6DQAhG70JcNmpceoPewc1kxL84wwQw1RjvXqzR7sjj%2Bx7skIkfMDioYVBNdVFMQjG%2FoCgx5sR9IR%2FPHl6Aq5%2BXFi3dNPkkmaFJPNQ7JMeCfE6yRXWrRXRLKJ7HpBZTMdS5WfAFLXpiZ3KBWnnBw9aOe1QroVRlVVphJhJ8KmGMY7edek8hMYr8Vy728508zBWoXU6w%2Fz4wpwutCSO9oyJTeo7e0qv5BTnH4YgMAQAA

Notice how FileSystemError is used for the class itself but also for the decorated and un-decorated variables. Can we re-open this issue please?

@swc-bot
Copy link
Collaborator

swc-bot commented Oct 16, 2022

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.

@swc-project swc-project locked as resolved and limited conversation to collaborators Oct 16, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

Successfully merging a pull request may close this issue.

3 participants