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

Redefine type always emit Object design:type #31

Open
FranckBontemps opened this issue May 4, 2020 · 5 comments
Open

Redefine type always emit Object design:type #31

FranckBontemps opened this issue May 4, 2020 · 5 comments
Labels
babel limitation Issues generated by babel current implementation

Comments

@FranckBontemps
Copy link

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.

@FranckBontemps FranckBontemps changed the title Redefined primary type Redefine type May 4, 2020
@FranckBontemps FranckBontemps changed the title Redefine type Redefine type always emit Object design:type May 4, 2020
@leonardfactory leonardfactory added the bug Something isn't working label May 11, 2020
@leonardfactory
Copy link
Owner

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
Copy link

namnm commented May 28, 2020

@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
Copy link
Owner

leonardfactory commented May 28, 2020

@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
Copy link

namnm commented May 28, 2020

@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
Copy link
Owner

leonardfactory commented May 28, 2020

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.

@leonardfactory leonardfactory added babel limitation Issues generated by babel current implementation and removed bug Something isn't working labels Mar 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
babel limitation Issues generated by babel current implementation
Projects
None yet
Development

No branches or pull requests

3 participants