JohnWeisz / TypedJSON

Typed JSON parsing and serializing for TypeScript that preserves type information.
MIT License
604 stars 64 forks source link

URGENT ! TypeThunk broke everything for Angular Production Builds #176

Open amoscatelli opened 3 years ago

amoscatelli commented 3 years ago

When compiled with production flags (optimization, aot, buildOptimizer to true) TypedJSON is now broken for Angular since TypedJSON 1.7 version (and also 1.8 RC of course).

The problem is the introduction of TypeThunk :

https://github.com/JohnWeisz/TypedJSON/blob/fd436e1d221fd16f45117b1739678e3d7a7e73b0/src/json-array-member.ts#L77

To explain the issue I must clarify that when compiled without productions flags, Angular turns this :

@jsonObject export class FindCriteriaDTO { }

into this :

var FindCriteriaDTO = /* @class / function () { function FindCriteriaDTO() {}

FindCriteriaDTO = __decorate([typedjson_1.jsonObject], FindCriteriaDTO); return FindCriteriaDTO; }();

With production flags this is compiled into this :

var FindCriteriaDTO = __decorate([typedjson_1.jsonObject], function() {});

typeThunk() function calls the MaybeTypeThunk function, that is, the __decorate function with anonymous function parameter and that returns undefined.

With 1.6 version that was not a problem since TypeThunks were not used :

https://github.com/JohnWeisz/TypedJSON/blob/01fda862b24fc874a7712346a77fcf08429a6548/src/json-array-member.ts#L74

This is urgent since it breaks production builds !

Please let me know as soon as possible Thank you

amoscatelli commented 3 years ago

@Neos3452 please look at this

amoscatelli commented 3 years ago

I confirm also this happens for empty classes. If I put a dummy parameters it is compiled to this :

        var FindCriteriaDTO = function() {
            function FindCriteriaDTO() {}
            return __decorate([typedjson_1.jsonMember, __metadata("design:type", String)], FindCriteriaDTO.prototype, "dummy"), __decorate([typedjson_1.jsonObject], FindCriteriaDTO)
        }();
Neos3452 commented 3 years ago

Hi @amoscatelli, thanks a lot for this report. I looked into it today, and unfortunately, this is a big oversight on the API side. I will need to change it, but this is not an easy task. For the time being, in your case, it should also be enough to add a dummy function inside your class.

Can you also tell me the javascript version target for typescript in your project?

MatthiasKunnen commented 3 years ago

I'm looking into a patch. Can you give us the content of your angular.json and tsconfig.json?

MatthiasKunnen commented 3 years ago

So far I'm unable to reproduce your issue.

I've compiled with the following settings:

"optimization": true,
"outputHashing": "all",
"sourceMap": true,
"extractCss": true,
"namedChunks": false,
"aot": true,
"extractLicenses": true,
"vendorChunk": false,
"buildOptimizer": true,

with production: true in the environment file and the following works fine:


@jsonObject
export class Empty {
}

@jsonObject
export class NoFactory {

    @jsonMember
    foo: string;
}

@jsonObject
export class FactoryOptions {

    @jsonMember({
        preserveNull: true,
    })
    foo: string;
}

@jsonObject
export class Thunk {

    @jsonMember(() => String)
    foo: string;
}

console.log(TypedJSON.parse({foo: 'bar'}, Empty)); // e {}
console.log(TypedJSON.parse({foo: 'bar'}, NoFactory)); // e {foo: "bar"}
console.log(TypedJSON.parse({foo: 'bar'}, FactoryOptions)); // e {foo: "bar"}
console.log(TypedJSON.parse({foo: 'bar'}, Thunk)); // e {foo: "bar"}

Please provide us with a minimal reproduction which should contain at least the following:

amoscatelli commented 3 years ago

Class on which the issue occurs :

@jsonObject export class FindCriteriaDTO { }

Classes on which the issue does not occur :

@jsonObject export class FindCriteriaDTO { @jsonObject dummy: string; }

OR (both of these workaround work for me)

export abstract class FindCriteriaDTO { }

Code which calls these classes :

        new TypedJSON<S>(
            requestConstructor
        ).stringify(request),
        new Map([
            ['Content-Type', 'application/json']
        ])

Where requestConstructor is this :

@jsonObject export class FindRequestDTO { @jsonArrayMember(FindCriteriaDTO) criterias: FindCriteriaDTO[]; @jsonArrayMember(FindOrderDTO) orders: FindOrderDTO[]; @jsonWrappingNumberMember() quantity: number; @jsonWrappingNumberMember() offset: number; }

Result of ng version :

PS C:\Users\amoscatelli\Documents\Visual Studio Code\optoplus-view> ng version

 _                      _                 ____ _     ___
/ \   _ __   __ _ _   _| | __ _ _ __     / ___| |   |_ _|

/ △ \ | ' \ / | | | | |/ _ | '| | | | | | | / _ | | | | (| | || | | (| | | | || |_ | | // __| ||_, |_,||_,|| __|__|| |/

Angular CLI: 11.2.1 Node: 15.9.0 OS: win32 x64

Angular: 11.2.1 ... animations, cdk, cli, common, compiler, compiler-cli, core ... forms, language-service, material, platform-browser ... platform-browser-dynamic, router, service-worker Ivy Workspace: No

Package Version

@angular-devkit/architect 0.1102.1 @angular-devkit/build-angular 0.1102.1 @angular-devkit/core 11.2.1 @angular-devkit/schematics 11.2.1 @angular/flex-layout 11.0.0-beta.33 @angular/pwa 0.1102.1 @schematics/angular 11.2.1 @schematics/update 0.1102.1 rxjs 6.6.3 typescript 4.1.5

Zipped angular and tsconfig :

angular.zip

typedjson version used to reproduce the issue :

both 1.7 and 1.8 rc 1

1.6 works correctly

amoscatelli commented 3 years ago

I want to stress out that if I remove the jsonObject annotation from the empty class everything works correctly (so this is a possible workaround for my case), since the __decorate function won't be called on the anonymous function.

amoscatelli commented 3 years ago

My advice to reproduce the issue is to check the compiled js first. First you reproduce something like this in your compiled js :

var FindCriteriaDTO = __decorate([typedjson_1.jsonObject], function() {});

If you don't have this, you won't reproduce it.

MatthiasKunnen commented 3 years ago

I've tried but still can't reproduce. Please fork this repository: https://github.com/MatthiasKunnen/typedjson-angular-bug-176 and make a commit that changes the necessary settings and has a clear failure

MatthiasKunnen commented 3 years ago

We know what the issue is, no need for further reproduction.

MatthiasKunnen commented 3 years ago

The issue is that a build step such as compiling TS with target <= ES5 turns the class into an anonymous function. We check whether a function is anonymous to determine whether it is a thunk or not.

The issue will be need to be addressed although it is not yet sure how but in the meantime you could pass a thunk or change your target to "ES2015" or higher. I saw that you don't have a target in your tsconfig which means that you are targeting ES3 by default.

amoscatelli commented 3 years ago

As far as I know, by default, the target is es2015 when using recent Angular.

Anyway I already found a satisfying workaround for me.

I just wanted to be sure you are going to address the issue. Thank you.

amoscatelli commented 3 years ago

https://angular.io/guide/typescript-configuration

amoscatelli commented 3 years ago

I was wrong, those entities were coming from a non angular library. So, they were es3 compiled indeed.

I confirm @MatthiasKunnen analysis. Compiling with ES2015 everything works out.