search

elnabo / json2object

Type safe Haxe/JSON (de)serializer
MIT License
66 stars 17 forks source link

Macro generating redundant source files #86

Open paulsgcross opened 1 year ago

paulsgcross commented 1 year ago

Due to the numbering scheme that json2object employs, it would seem that redundant source files are being generated. For example, for a JsonParser that parses an object such as MyObject, it is possible (after enough builds) for the macro to generate JsonParser_0 and JsonParser_2 that both perform the exact same role. This means, after enough compilations, we can have as many as hundreds of redundant parsers. While performing a complete rebuild circumvents this, it is not ideal and could also lead to source code conflicts if we choose to only rebuild.

My suggestion would be to name the parsers not by integer, but by the type they operate on (i.e. JsonParser_MyObject), while the macro can throw in a quick "if type exists" check using the object name to establish if it needs to recreate the type or not.

elnabo commented 1 year ago

Hello, my memory is a bit fuzzy about this but I think your suggestion was the original design.

However that way, the parser were never regenerated by the compilation server even in case of changes.

I agree that the current system is suboptimal, but I don't see an elegant solution that doesn't come with worse drawbacks

paulsgcross commented 1 year ago

Ah, I had wondered if it was the original design -- thanks for clearing that up.

Out of curiosity, do you remember what it was in the original design that caused issues? I know that Context.registerModuleDependency can be used to force re-typing if specific files change.

elnabo commented 1 year ago

I think the problem was that once we defined a type, if we tried to define an other one with the same name, we would often have error like "Type name JsonParser_1 is redefined from module JsonParser_1" in the compilation server.

paulsgcross commented 1 year ago

When defining the type via the macro, did you also make sure to include the package path in it's name?

For example you transform path.to.MyObject into JsonParser_path_to_MyObject. I have done this before and it's a sure-fire way to prevent naming conflicts like the one you mentioned above. Have I understood correctly?

elnabo commented 1 year ago

Yes we did do that and it caused other problem due to name length I think.

But the conflict I mentioned should appears as long you as try to define twice a class with the same name in the same compilation server.

paulsgcross commented 1 year ago

My second thought, if name length is an issue, would be to hash the object name and path into something more managable. Happy to fork off this repo and give some ideas a go and see how it plays out.