Hive_generator built value support #533
Conversation
* Implemented: Built values, BuiltLists, BuiltMaps and BuiltSets reading and writing. * TODO: EnumClass support
built_collections
|
I noticed i did not implement support for custom builders, so i will do it before unmarking as draft |
|
Actually, custom builders can have nested builders or regular built values, i think supporting every usecase will be a pain, and letting the user implement the adapter for these edge cases may be a better idea imo |
|
Can we publish it as an independent package? Because |
|
hmm, i dont know. The hive_generator does not generate code which depends on built_value or built_collections unless the user explicitly implements |
|
I'll check if publishing as a separate package which depends on built_value and built_collection is possible |
|
I tried it and managed to implement this feature as an separate package after exposing a bit of ClassBuilder behavior and creating an CreateBuilder field on TypeAdapterGenerator which creates the Builder from the class, setters and getters. Soo, i'll close this PR and open a new one with this exposing, and then i maintain the packages (hive_built_value and hive_built_value_generator), right? |
Can I know what's difference between |
Yess, hive_built_value is a runtime package with no code which depends on hive, built_value and built_collection, as you suggested:
And hive_built_value_generator is the dev_dependency, which depends on the hive_built_value package instead of hive and generates code that works with So, for an user to use hive with built_value classes, he will use hive_built_value instead of hive in the dependencies and hive_built_value_generator instead of hive_generator in the dev dependencies. Is this what you proposed with your suggestion or did I misunderstood? #533 (comment) |
It's alright. I think we can merge It's actually not necessary to make everything perfect. I just want to make sure packages don't confuse future users and won't break exists structure. Maybe @leisim have any ideas about publishing multiple packages vs bundling generators into same package. |
|
Hm alright. So, we should generate normal code for the normal packages, and generate code that may use built_value and built_collection only in packages that import hive_built_value as a runtime dependency then, preserving the current behavior and adding the functionality, right? |
😂 I'm so confused. You can ignore everything I said previously and do whatever you think is right. |
|
Hm, okay, ill finish implementing it then. Sorry for all the confusion. |
* Separate logic into regular and built methods * Expose cast, buildReadConstructor and convertWritableValue * Will be used to separate into two classes
* Split the base behavior into _ClassBuilderBase and the built_value behavior into ClassBuilder. * I think this is less cluttered, as the built_value behavior will get more complex with the support for custom Builder classes and @BuiltValue annotation
* We will need to use more type checkers, and this will get out of hand. * Besides, this is an dev_dependency only, so it isnt that big of a deal
builder is being generated
|
i've been testing it a bit with some classes from https://github.com/google/built_value.dart/blob/master/end_to_end_test/lib/values.dart and i think the only missing thing now is support for nested built_collection s |
* well, this is embarassing...
* Only for built_value classes for reducing the amount of generated code to a reasonable level
* bad idea to do rn. This reverts commit ff1181e.
|
Nesting built_collections is working right now, both for reading and writing. NestedThingsValue generates the following code: NestedThingsValueAdapter |
|
All these changes should not affect regular classes, because in each overriden method, if the value is not built, the super method is returned, but some testing is needed, as this is critical. Do you think i should start adding some tests to hive_generator, or i should test a bit more manually in my repo for this pull request? |
|
uhh, so, may i rebase it, and test it a bit more manually, so it can get merged? |
Sure. |
|
@KalilDev Any chance that this PR could be adapted for null safety? |
So, now TypeAdapter for Built value classes is correctly generated, the reading and the writing work fine. Support for BuiltMap, BuiltList and BuiltSet was added too, they are written as an regular List/Map and restored using (Set/List/Map)Builder, which does all the casting and validation. EnumClass is stored using the name, just like the regular built_value serialization, as the name can be overriden using an annotation. Yeah, i think thats it! Here is an example