dipscope / TypeManager.TS

Transform JSON strings or plain objects into JS class instances.
https://dipscope.com/type-manager/what-issues-it-solves
Apache License 2.0
24 stars 3 forks source link

Add support for TypeScript v5 decorator syntax #15

Open Offlein opened 1 year ago

Offlein commented 1 year ago

Describe the bug

Out of the box, my PhpStorm (or WebStorm) IDE throws a TS1238 error on the @Type decorator when called as an expression, and a TS1240 error ("Unable to resolve signature of property decorator when called as an expression. Argument of type  undefined  is not assignable to parameter of type  Object") on the @Property decorator.

I have been having a lot of difficulty recently understanding how PhpStorm validates syntax across our monorepo, but I get this error when my "Node Interpreter" is set to my host machine's (version 20.5.0), and it goes AWAY when I set it to use Node within this app's docker dev environment (node version 18.16.0). It comes BACK if I set it to use a different container that uses node 16.13.1. On all of the above it's set to use Typescript version 5.0.4, and I don't understand why the Node version matters at all in that case.

Either way, I do see that @Type's signature is:

export declare function Type<TType>(typeOptions?: TypeOptions<TType>): ClassDecorator;

And indeed that accepts one optional parameter.

I am not intimately familiar enough with TypeScript to understand why the runtime will invoke it with 2 parameters, but is that a legitimate issue, and do these decorators' signatures need to be updated to handle that 2nd parameter?

image

Thanks!

dpimonov commented 1 year ago

Hi Offlein, thanks for creating an issue. It is known to me but it is good to have a ticket for it.

The issue is in TypeScript version. In v4 decorator syntax was different as in v5 and these 2 are incompatible. In version 5 well typed decorators were introduced for Class, Property and Method but not for Parameter. It will be introduced in later TypeScript versions as documentation says.

Migration of this library to a new decorator syntax cannot be fully done without Parameter decorator as it is used for immutable types and DI. Until decorator topic will be fully covered by TypeScript team there are several options:

1) Try to enable --experimentalDecorators option in tsconfig if possible. This should switch decorator syntax to an older version. This is a valid option to solve a compatibility issue from official docs.

image

2) Use declarative configuration instead of decorators. It provides absolutely the same set of features. As we are not using decorators - there will be no error in v5.

import { TypeManager, TypeMetadata, TypeConfiguration } from '@dipscope/type-manager';

export class User
{
    public name: string;
    public email: string;
}

export class UserConfiguration implements TypeConfiguration<User>
{
    public configure(typeMetadata: TypeMetadata<User>): void 
    {
        typeMetadata.configurePropertyMetadata('name')
            .hasTypeArgument(String);

        typeMetadata.configurePropertyMetadata('email')
            .hasTypeArgument(String);

        return;    
    }
}

TypeManager.applyTypeConfiguration(User, new UserConfiguration());

3) Switch to TypeScript v4.

When new Parameter decorator syntax will become available in TypeScript v5 - then we are going to provide a propper update.

Offlein commented 1 year ago

Thank you @dpimonov. I really appreciate you taking the time out to explain. I believe I understand this issue now.

Further complicating things is the fact that, I believe, PhpStorm does not seem to correctly handle a base tsconfig.json file that branches off into two other files. experimentalDecorators did not give me the expected fixes until I worked that out.

It feels to me like we most-benefit from using TS 4.X instead of 5.X, actually, so I may prefer to keep our environment configured to use the earlier version with the experimentalDecorators enabled.

The only negative result seems to be that all the properties on my class are flagged with TS2564: Property  {...}  has no initializer and is not definitely assigned in the constructor., which of course is not a TypeManager-specific issue, but did not occur before.

I guess I'm curious if that's because TypeManager's decorators somehow implied to TS that those values will be only initialized via deserialization, or whether it was just ignored for other reasons. I know I can turn off strictPropertyInitialization or !: the properties but I'd love to know if that's expected or something else.

Thanks so much for the help. I think your library is very slick.

dpimonov commented 1 year ago

Thanks for the feedback!

The only negative result seems to be that all the properties on my class are flagged with TS2564.

This is expected when strictPropertyInitialization is on. All class properties should be assigned via constructor or marked as not nullable when this option is on. It is not TypeManager issue and purely dependent from your config. The library provides a way to declare classes when this option is on.

@Type()
class UserStatus
{
    @Property(String) public readonly name: string;
    @Property(String) public readonly title: string;

    public constructor(@Inject('name') name: string, @Inject('title') title: string)
    {
        this.name = name;
        this.title = title;

        return;
    }
}

However they will be deserialized properly even without property injections. So basically you have to decide if you want TypeScript check properties for initialization or not. In general TypeScript v5 is relatively new so maybe it still does not recognize all available options. Thats why no error occured before.

Offlein commented 1 year ago

Yep, that's what I thought. Thanks for your time and generosity addressing an irrelevant issue. Really wonderful support.

dpimonov commented 7 months ago

When new Parameter decorator syntax will become available in TypeScript v5 - then we are going to provide a propper update.

Seems like it will not happen in the close time. I've added required changes to support modern class, property and accessor decorators starting from v7.2.1.

It means that Type and Property decorators should now work with modern syntax. However Inject decorator will no longer be aplicable until there will be some kind of support from TypeScript. One can use declarative configurations for this part if required.

I am going to keep this issue open for future updates about modern decorator syntax.