-
-
Notifications
You must be signed in to change notification settings - Fork 5
Create augmentation prototypes of test models using augment keyword and *.codable.dart part files
#9
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…ric and polymorphism/complex examples
|
This is really cool, I need to take a deeper look when I got time. |
| late String? name; | ||
| late int age; | ||
| late bool isActive; | ||
| late DateTime? signupDate; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nullable options should not be late, since they may simply not appear at all.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch @TekExplorer ! In the case of nullable fields within the model the code generator should not generate late modifiers because the runtime check for initialization (forced by using late) would generate an exception when the field had not been initialized explicitly to null.
This block should look like this
late String id;
String? name;
late int age;
late bool isActive;
DateTime? signupDate;
Uri? website;
So name and signupDate auto initialize to null (and could possible be changed to something else during the decode).
@schultek Thoughts ?
|
i like the usage of augmentations, but i also think that the implementation as it is is also super cluttered. i suggest combining the implementation details that arent relevant, or even remove them entirely. stuff like toString, ==, hashcode, can be implied since these are examples. If you do not want to remove them, at least put them in the same augmentation. additionally, you switch from field-first to field AND constructor at the inheritance part. i would suggest taking the freezed route and going constructor-first, or just have constructors everywhere. that may be more consistent if you choose to remove the "data class" code Its just a bit jarring. |
|
@TekExplorer - The reasons I have multiple In the examples I ended up going with but I could also imagine something like in which case the annotations would each be generating there own code blocks (and therefor separate With the syntax that I went with it should be possible to combine the different I was just trying to exercise the We suspect will also want to support separate |
|
yes, but i dont think its a good idea to confuse the examples with out-of-scope features. the point of this is Codable, not equatable and tostring. |
|
I understand the point you are making, but my intentions here were not to create simplified examples. Some of the original examples included equitability and toString functionality, so these augmentation examples do as well. I will comment that I do think it would be worthwhile for the Codable annotations to be able to optionally implement this functionality to it's Codable augmentations. |
Okay
Eh, I don't really agree. Notice that json_serializable doesn't do that, while freezed does. The difference being, json_serializable, like Codable, is purely about serialization, which may not necessarily be "data class". You can have mutable, serializable classes. Freezed, however, is focused on data classes, which usually implies serialization (not the other way around) and doesn't even do it itself - it just supports it with json-serializable. Let Codable focus only on what it's intended to do. |
Within the example code you can find (in comments) purposed annotations such as These arguments would be optional. I personally would like Codable to generate this repetitive (and laborious and time consuming) code for me, as it continually changes as the models change (just as the serialization code). I will also restate that as everything in this PR these are just ideas I extrapolated from @schultek 's original RFC code and things such as the annotation syntax were specified as to be clear in the example code and spark discussion (such as this). ps. |
This PR adds a subdirectory
augment_testwithintestthat is a copy of thetestdirectory, with the only changes being that the model classes have been edited to use the forthcoming dart augmentation capabilities/experimental feature.My intention here is not to have this PR merged, but to spur discussion and to discover bugs and limitations in the current dart
augmentationexperimental implementation.I am imagining here the code that a possible
@Codable()annotation, implemented via a build_runner builder, might automatically produce,I did discover both crashes and bugs that can be triggered within the analyzer when using the
augmentkeyword. Fortunately, these are only triggered by usingaugmentto create the generic and polymorphism models.So far I have filed dart-lang/sdk#60040 and dart-lang/sdk#60039
The augmentation code that causes the analyzer crashes has been commented out for now and left as the original examples defined the classes (to leave the analyzer intact/crash free within the VSCode environment). The code has been commented to indicate what causes the crashes (or incorrect analyzer errors) and the temporary (unaugmented) code is commented/highlighted.
(I still need to isolate/create standalone version of the crashes and polymorphism errors, see README.md within the
augment_testdirectory for more info and references to where these problems occur.)The only modified files within the
augment_testdirectory areI included all of the other test files in the
augment_testdirectory so that the tests could be used to verify that theaugmentversions work identically. (Currently they CANNOT be run because the dart compiler has an error preventing any use of theaugmentkeyword from compiling (or even formatting)) dart-lang/sdk#60039