@serializable: Error squiggles in VSCode

[krisrok]

Describe the bug 💬

VSCode is complaining about usage of @serializable on non-primitive targets like Object3D:

This renders almost every file/tab with red font because it has this error.

Changing the returned function to also accept any and transforming it to a string if necessary seems to work:

    return function (_target: any, _propertyKey: string | any) {
        // this is important so objects with inheritance dont override their serialized type 
        // info if e.g. multiple classes inheriting from the same type implement a member with the same name
        // and both use @serializable() with different types 
        if(typeof(_propertyKey) != 'string')
            _propertyKey = _propertyKey.toString();
        if (!Object.getOwnPropertyDescriptor(_target, '$serializedTypes'))
            _target["$serializedTypes"] = {};
        const types = _target["$serializedTypes"] = _target["$serializedTypes"] || {}
        types[_propertyKey] = type;
    }

Squiggle gone, yay:

Operating System 👩‍💻

Windows

What browsers are you seeing the problem on? 🏄‍♂️

No response

Editor Version 🎲

2020.3

Needle Exporter version

3.13.0-pre

Project Info (Unity only) 📜

No response

Steps to reproduce 🔢

1. Open a .ts class using @serializable

Reproduction ♻

No response

Discord or Discussion link

No response

Validations 🩹

• Follow our Code of Conduct
• Read the docs.
• Check that there isn't already an issue that reports the same bug to avoid creating a duplicate.
• Make sure this is a Needle-Engine-specific issue. For example if it's a Unity related bug, it should be reported to Unity instead.
• Check that this is a concrete bug. For Q&A open a GitHub Discussion or join our Discord Chat Server.
• The provided reproduction is a minimal reproducible example of the bug.

[marwie]

Hey, which version of typescript are you using?

[krisrok]

Oh is this a fault on my end only? Sorry, I'm new to this whole ecosystem.

Anyway, this is what i found:

> npm ls typscript
my-needle-engine-project@1.0.0
├─┬ @needle-tools/helper@1.1.0
│ └─┬ @needle-tools/needle-component-compiler@1.10.3
│   └── typescript@4.9.5
└── typescript@5.2.2

[marwie]

Thanks, I havent tested it with typescript 5 yet.

Can you try if changing the type to string | symbol works for you as well?
https://devblogs.microsoft.com/typescript/announcing-typescript-5-0/#more-accurate-type-checking-for-parameter-decorators-in-constructors-under-experimentaldecorators

[krisrok]

Thanks, I havent tested it with typescript 5 yet.

Oh? That's what gets installed automatically using the exporter though? I did not (knowingly) deviate from the defaults.

Can you try if changing the type to string | symbol works for you as well? https://devblogs.microsoft.com/typescript/announcing-typescript-5-0/#more-accurate-type-checking-for-parameter-decorators-in-constructors-under-experimentaldecorators

Nope:

[marwie]

Ah sorry I wasnt aware that it was updated in the vite template. So that can't be it

What do you get when you change it to any to make typescript happy and if(typeof _propertyKey !== "string") console.log(typeof _propertyKey, _propertyKey) ?

[krisrok]

Interesting: Does not log anything. I can set a breakpoint there and it hits (after un-ingoring the file). Is there some magic going on?

[marwie]

Ah you need to edit the file in lib/engine (the .js file) and not in src/engine

[hybridherbst]

Looks like in TS5 this parameter is a ClassFieldDecoratorContext instead of a string. My understanding would be that there's different TS versions conflicting, one with a config of experimentalDecorators (like we're using, with the string parameter) and one without.

This issue would likely go away if we switch to TS5 entirely and drop support for experimentalDecorators, instead using the non-experimental ones with that context.

I'm not sure if we can use ClassFieldDecoratorContext as alternative type or if that's just not available in TS4, which would make it seem like "any" and casting to string is the right way for now...

Here are some decorator usage notes for TS5:
https://devblogs.microsoft.com/typescript/announcing-typescript-5-2-beta/#decorator-metadata

https://github.com/microsoft/TypeScript/blob/3ade5022d7f287fdaf61c9e6deed32a3ffb60bc4/src/lib/decorators.d.ts#L326

And the info in the PR that added it is much better than the current docs:
microsoft/TypeScript#50820

[marwie]

We'll add a fix for this in the next update by supporting both the old and the new decorator syntax:

_propertyKey: string | { name: string }

a call to toString() in this case doesn't make sense since we need the decorated field/property name here. The new method argument (ClassFieldDecoratorContext) has that inside the name property so we have to properly handle both types and extract the information necessary.

That being said our package will continue using experimentalDecorators as long as it's officially supported by typescript to not break backwards compatibility - according to their announcement it won't be removed anytime soon.