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

Decorators failed to resolve metadata type correctly on getter #5252

Closed
stevefan1999-personal opened this issue Jul 19, 2022 · 8 comments
Closed

Comments

@stevefan1999-personal
Copy link

stevefan1999-personal commented Jul 19, 2022

Describe the bug

Ditto to title

Input code

import { Entity } from "typeorm/decorator/entity/Entity"
import { Column } from "typeorm/decorator/columns/Column"
import { PrimaryGeneratedColumn } from "typeorm/decorator/columns/PrimaryGeneratedColumn"

@Entity()
export class Post {
    private _title: string

    @PrimaryGeneratedColumn()
    id: number

    set title(title: string) {
        // this._title = "!" + title + "!"; // if you'll do "append" like this, you won't get expected results, because setter is called multiple times
        if (title === "hello") {
            this._title = "bye"
        } else {
            this._title = title
        }
    }

    @Column()
    get title(): string {
        return this._title
    }
}

Config

{
  "jsc": {
    "externalHelpers": true,
    "keepClassNames": true,
    "parser": {
      "decorators": true,
      "preserveAllComments": true,
      "syntax": "typescript",
      "tsx": false
    },
    "target": "es2019",
    "transform": {
      "decoratorMetadata": true,
      "legacyDecorator": true
    },
    "loose": false,
    "minify": {
      "mangle": false,
      "compress": false
    }
  },
  "module": {
    "type": "commonjs"
  },
  "minify": false
}

Playground link

https://play.swc.rs/?version=1.2.218&code=H4sIAAAAAAAAA42Q3UrDQBCF7wN5h3Fv2qKY%2B0qhIOJt30DSZNoO7k%2BYnahB%2Bu7OJqZNpRSXwCbM%2Bc45GXJNYIFvePFC0sERdhwcGOkaDOyKGqvApQQusBcUg87kGY3kc7Ct8zfIqhfEYhBO0Q2TK7l7RY8qxfrfVtdBtc6z9dBwvsgz%2FOpzKlvGCJsQNTLPQE%2FD9KEYvKnS4hKiMPl9otN0fd09OaYx1Uvwrdsij0BEgd5pfuG3GOPSKQqQA8XHIRJWYO4M3A%2BY3vr1lDS0gy60M2uhDmDKpkFfG7D0jj3%2BkKbwGfxMYK%2Bp%2BodYaUNgjK0VnW%2BxKtuIqZMgA0WoSmtV4XROjYYJOYznYpo4%2F%2B200lYHtDaYi%2Brp%2FOm%2B7dCcBUdAq5E3kf6eIMPr8bTzyx3vTxtdjNuc2jNKy36acLLT5we3xAvJ1AIAAA%3D%3D&config=H4sIAAAAAAAAA1WOsW7DMAxE%2F4Wzh6ZbvRXp0KX9B0I%2BG04lUSCZIIaRf6%2BU1DE68t7d8VY6WaB%2BJVwdmjl%2BIhaoUe96Rkc%2FQDlGNvvmhKdaWA3aYgOCKLvsiaKo7IL3GI%2BSErI%2FkS3Z%2BUo9%2BVJgQefi1JFblUaOhls9WCd4tcBeXw5vDStnG0XTv3dfcB7YeauOmDgsHxt%2ByLUvihj%2B6jtKc57HpRUlzlPcQZDUdts2pCaTDOfmWO9r66LqSZJPRre96OH%2BBQMMNOhFAQAA

Expected behavior

Matches the output in official TSC compiler:
https://www.typescriptlang.org/play?noEmitHelpers=true&declaration=false&importHelpers=true&target=6&jsx=0&module=1&stripInternal=false&preserveValueImports=false&ts=4.7.4&downlevelIteration=true#code/JYWwDg9gTgLgBAbzgUQHY2DAnnAvnAMyghDgCJswBTaEAegBMqBjaAQxmjqvUyzrQZsZALAAoUJFiI4AYQgAbAK4hUeQsVIUs1WoxbtOUOq2WqAznXlnUoieGjwkABSig2ULAHEeVKByoGaxU1fCISckoaKHomVn8jE0UQy1d3Tx9UPwCg5NU7cQABQT4ACgBKcSoADyl4ZgU2c3M4ZwhzJ3E4brgwNwA3ALgAfSEFKgAuOA63VABzcS6ewrSQD29fBMDg1Qql7uAGKdQVACM-RbEe6ap4MapS+6mZ4HnyxH3ruDo6OBgAC2A5gAdKNMOM4ABecgAQjIcAA1H9wVREbCyABub6-YAEOBYCBKADkCgUcAYEHIbDA1FQDHhCmAAGtUQCgQAafGEuAAdwgqCJ8DmtzgNWozBggTgUCo5iUChg5k552YbCU5lRGpgkqgcCBcFVpKlIHlGDAEIwIFln2uuLgjxRUMh0LI-yopIgZHeCBtX26bJBYJgEJdpywVDsfu6+HdGo+Vyj1wDoPuUORwaovujn1wl2uhR2qD2Ce6wruKIqzxgszm8cTMpgSigamTQfGOfEubEQA

Actual behavior

Emits a wrong type deduction:

_tsDecorate([
    (0, _column.Column)(),
    _tsMetadata("design:type", Function),
    _tsMetadata("design:paramtypes", [])
], Post.prototype, "title", null);

vs

tslib_1.__decorate([
    (0, Column_1.Column)(),
    tslib_1.__metadata("design:type", String),
    tslib_1.__metadata("design:paramtypes", [String])
], Post.prototype, "title", null);

Notice both design:type and design:paramtypes mismatches.

Version

1.2.218

Additional context

Related: #3998

@stevefan1999-personal stevefan1999-personal changed the title Decorators failed to resolve type correctly on getter Decorators failed to resolve metadata type correctly on getter Jul 19, 2022
stevefan1999-personal added a commit to stevefan1999-personal/typeorm that referenced this issue Jul 19, 2022
blocked by swc-project/swc#5252

Signed-off-by: Steve Fan <29133953+stevefan1999-personal@users.noreply.github.com>
@kdy1 kdy1 added this to the Planned milestone Jul 20, 2022
@gaunap

This comment was marked as spam.

@hbina
Copy link

hbina commented Sep 13, 2022

@kdy1 do you have a pointer to get me started on this issue? I grepped for _tsDecorate and can only find TS files so I am not sure where to look for it.

@hbina
Copy link

hbina commented Sep 18, 2022

@stevefan1999-personal Can you elaborate exactly what is expected here? I currently have made this change but after some testing this is not enough.

hbina@akarin ~/g/s/b/swc_cli (main)> git diff -- :/crates/swc_ecma_transforms_proposal/src/decorators/legacy/metadata.rs
diff --git a/crates/swc_ecma_transforms_proposal/src/decorators/legacy/metadata.rs b/crates/swc_ecma_transforms_proposal/src/decorators/legacy/metadata.rs
index 2b526f1b80..078e575043 100644
--- a/crates/swc_ecma_transforms_proposal/src/decorators/legacy/metadata.rs
+++ b/crates/swc_ecma_transforms_proposal/src/decorators/legacy/metadata.rs
@@ -136,8 +136,10 @@ impl VisitMut for Metadata<'_> {
         }
 
         {
-            let dec = self
-                .create_metadata_design_decorator("design:type", quote_ident!("Function").as_arg());
+            let dec = self.create_metadata_design_decorator(
+                "design:type",
+                serialize_type(None, m.function.return_type.as_ref()).into(),
+            );
             m.function.decorators.push(dec);
         }
         {

For example, if you have decorators set for both the getter and setter, tsc will only generate 1 decorator which seems to be a combination of both (function that takes 1 T and return T). Note that you don't need to have the same decorator on both of the getter/setter, one of them is enough.

https://www.typescriptlang.org/play?downlevelIteration=true&noEmitHelpers=true&declaration=false&importHelpers=true&target=6&jsx=0&module=1&stripInternal=false&preserveValueImports=false&ts=4.7.4#code/JYWwDg9gTgLgBAbzgYQgGwK4gHZwL5wBmUEIcARDAJ5gCm0IA9ACa0DG0AhjNIx5jgDOjVAOzkAsAChptAB6RYcNmk6DBcAAoRB8BNLiG4YKMABu3WnAD6MYDDS0AXHF2nsAc2kGjHbG4w2HigACjsHZ1cYdw8ASkQfIyMYAAtgQQA6W3tHOABeOHDHAG5E-G8ZKSSAAVEsbBDYso9aeCLaRpc3YE8EqqSjKFaMKFxU9Iz2srwKmrqcRrLBVsKcjvau6J64voHktMzsiPzViMmIAGUtz0X+wxnKvCA

class Post {
    constructor(title) {
        this._title = title;
    }
    get title() {
        return this.title;
    }
    set title(title) {
        this._title = title.toString();
    }
}
tslib_1.__decorate([
    (0, Column_1.Column)(),
    tslib_1.__metadata("design:type", String),
    tslib_1.__metadata("design:paramtypes", [String])
], Post.prototype, "title", null);

whereas we generate 2 decorators

_tsDecorate([
    (0, _column.Column)(),
    _tsMetadata("design:type", String),
    _tsMetadata("design:paramtypes", [])
], Post.prototype, "title", null);
_tsDecorate([
    (0, _column.Column)(),
    _tsMetadata("design:type", void 0),
    _tsMetadata("design:paramtypes", [
        String
    ])
], Post.prototype, "title", null);

@kdy1 kdy1 self-assigned this Sep 27, 2022
@kdy1 kdy1 removed their assignment Sep 27, 2022
@swc-bot swc-bot added the Stale label Oct 2, 2022
@swc-bot
Copy link
Collaborator

swc-bot commented Oct 3, 2022

This issue has been automatically closed because it received no activity for a month and had no reproduction to investigate. If you think this was closed by accident, please leave a comment. If you are running into a similar issue, please open a new issue with a reproduction. Thank you.

@swc-bot swc-bot closed this as completed Oct 3, 2022
@gaunap
Copy link

gaunap commented Oct 5, 2022

Having decorators on both setter and getter seems not to be allowed: "Decorators cannot be applied to multiple get/set accessors of the same name. (1207)". So I guess aborting compilation or ignoring the 2nd decoration and not handling this case should be ok.
Could this issue please be reopened?

@hbina
Copy link

hbina commented Oct 5, 2022

Does swc have a separate lint pass?

@swc-bot
Copy link
Collaborator

swc-bot commented Nov 5, 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 Nov 5, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

No branches or pull requests

5 participants