[gql_code_builder] fix: vars_create_factories handle BuiltList type #447
Conversation
LiLatee
force-pushed
the
fix_vars_create_factories_for_built_list
branch
from
213bd98 to
8f3a467
Compare
LiLatee
changed the title
| /// "$BuiltList" is a String "BuiltList<dynamic>" and | ||
| /// [g.returns!.symbol!] returns "BuiltList" | ||
| /// so we cannot use equality operator here. | ||
| final isBuiltList = g.returns?.symbol != null |
There was a problem hiding this comment.
can you also check the URL of the reference is from built_value, so we cannot have false positives here?
There was a problem hiding this comment.
Good call, I thought of doing that too to find Built Types at one point (when I wrote some of that code), but I think I didn't end up doing that as it was extremely hard to pin down the correct import.
For BuiltList s though, this would be checking for the built_lists import though, which doesn't change as often, right?
An alternative that might be worth exploring though, would be to check if all BuiltList instances are of type ListType and ListType is exclusively used for BuiltLists. Then you could "simply" add another check to L47
There was a problem hiding this comment.
I think the url of the Reference should be '"package:built_collection/built_collection.dart", I think this could just be added additionally to this check to avoid clashes with types from the schema, which theoretically also could contain symbols that contain the name BuiltList
| @@ -165,7 +165,7 @@ class SubscriptionData extends GraphQLSocketMessage { | |||
| int get hashCode => toJson().hashCode; | |||
|
|
|||
| @override | |||
| bool operator ==(dynamic other) => | |||
| bool operator ==(Object other) => | |||
There was a problem hiding this comment.
Is this an unrelated change?
There was a problem hiding this comment.
Yes. To be honest I cannot figure out why but the analyzer was failing
Your commit was the last one and then it was succeeding. Now it doesn't 🤷♂️
There was a problem hiding this comment.
Maybe because workflow uses dart:latest, so something in the Dart could change.
There was a problem hiding this comment.
yes, it's likely a new default lint rule in dart
There was a problem hiding this comment.
yes, this was the change dart-lang/sdk@aad9e57
There was a problem hiding this comment.
I couldn't find that 😅
Hi! 👋
I've tried to use that feature vars_create_factories and I've encounter an issue with arguments being lists. The parameter of a factory requires a
BuiltListthat is assigned toListBuilderand that is not allowed.@caffeineflo I've found some solution. Not sure whether it's the best way, but it works for me. So if you could take a look 🙏