leonardfactory / babel-plugin-transform-typescript-metadata

Babel plugin to emit decorator metadata like typescript compiler
MIT License
215 stars 17 forks source link

Redefine type always emit Object design:type #31

Open FranckBontemps opened 4 years ago

FranckBontemps commented 4 years ago

Hello,

I have an issue, when I define a type like this: export type Uuid = string;

The emit type is: Object

The full case:

export type Uuid = string;

function Decorate() {
    return (target: any, prop: string): any => {
        const propertyType = Reflect.getMetadata("design:type", target, prop);
        console.log('propertyType', propertyType); // emit the Object type
    }
}

class Foo {
    @Decorate()
    public bar!: Uuid;
}

and my babel config:

// babel.config.js
module.exports = {
    presets: [
        ["@babel/preset-env", { "targets": { "node": "current" }, "modules": "commonjs" }],
        "@babel/preset-typescript"
    ],
    "plugins": [
        "babel-plugin-transform-typescript-metadata",
        ["@babel/plugin-proposal-decorators", { "legacy": true }],
        ["@babel/proposal-class-properties", { "loose": true }],
        "@babel/proposal-object-rest-spread"
    ]
};

I don't know if it's a defect coming from my Babel configuration, or a limitation from @babel/preset-typescript.

leonardfactory commented 4 years ago

Unfortunately, this is a known issue caused by using babel. The pitfall is described in the README.

We don't have access to type aliases right now in the babel plugin, so we just emit typeof Uuid === 'undefined' ? Object : Uuid, and in this case since Uuid is just a type (and it's not present at runtime), Reflect.getMetadata will give you the wrong type.

I've started looking at some solutions, but I'm not sure I could deliver them in fast times since it requires a different approach than what is used right now in this project.

namnm commented 4 years ago

@leonardfactory Hi, instead of convert into Object, can we throw an error in the babel plugin so we would know this and change that type to something else?

And what is your suggestion to manually fix this btw?

leonardfactory commented 4 years ago

@namnm Throwing would cause an error even with defined types. The problem is exactly that we don't know what Uuid is: it could be a class or a type alias, and there is no way to get it.

The situation is, i.e. the following:

import { Uiid } from '...';

@UseEncode(Uuid)
class MyClass { }

now Uuid with TSC has more info attached since typings are evaluated by TS Checker. In babel instead you only get the info that Uuid is a scoped element with Uuid type. It could be a class type, and in this case it is perfectly fine to output it, but it could be an interface or alias, and in this case Uuid value is not reserved at runtime.

Edit: As a solution, I suggest to avoid using aliases in this situations and just fall back to plain strings. Babel TS support is pretty limited in this context. If this issue (and babel-ts flow) starts getting wider adoption, I'll start with a different implementation trying to get over the problem. See #30

namnm commented 4 years ago

@leonardfactory Thanks for the suggestion and quick reply. Just one more thing, I came here from the issue in the nestjs repo. I see that in the README this package can work well with Nest and TypeORM. Have you had any experiment project using this package for Nest? Is there any cautions that I need to take if I use this package in production with Nest and TypeORM?

leonardfactory commented 4 years ago

I've used it exactly with the same setup, but not in production, where we reverted to TypeScript only (no babel, but same toolkit). With TypeORM you should be careful to use @Column(), even if for plain cases it still works and for advanced cases you still need to declare it manually anyway. With NestJS it works properly but TS support is still better.