-
Notifications
You must be signed in to change notification settings - Fork 8
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
Dart client corrupts any dto's List properties in flutter web release mode (minification), and a suggested fix #756
Comments
What's the easiest way to get a repro of this issue? i.e. what's the original C# DTOs and what client Dart code can I run to repro this? |
Define these in C#: public class Foo
{
public List<Bar> bars { get; set; }
}
public class Bar
{
public string name { get; set; }
public string description { get; set; }
}
public class Baz
{
public string name { get; set; }
public string description { get; set; }
}
public class FooRequest : IReturn<Foo> { }
public class RawBazRequest : IReturn<List<Baz>> { }
public class IssueService : Service{
public Foo Get(FooRequest dto)
{
return new Foo
{
bars = new List<Bar> {
new Bar {
name = "bar item 1",
description = "item 1 description"
},
new Bar {
name = "bar item 2",
description = "item 2 description"
}
}
};
}
public List<Baz> Get(RawBazRequest dto)
{
return new List<Baz> {
new Baz {
name = "item 1",
description = "item 1 description"
},
new Baz {
name = "item 2",
description = "item 2 description"
}
};
}
} For this issue: For #721 : 'Bar': new TypeInfo(TypeOf.Class, create:() => new Bar()),
'Baz': new TypeInfo(TypeOf.Class, create:() => new Baz()),
'Foo': new TypeInfo(TypeOf.Class, create:() => new Foo()),
'List<Bar>': new TypeInfo(TypeOf.Class, create:() => new List<Bar>()), The In Flutter, call RawBazRequest and render the items in a list. I think this will fail on web or Android/iOS in debug too (I'm not sure, because I already have my workarounds). Build a release version of Android and web and try the same thing. This definitely fails. |
Then try the same with Dictionary responses, and generic class responses eg public abstract class AbsPagingResponse<T>{
public int TotalCount { get; set; }
public bool HasMore { get; set; }
public List<T> Items { get; set; }
}
public class PagingResponse : AbsPagingResponse<Foo>{
public string FooInfo { get; set; }
} These also don't work quite correctly due to problems in the codegen. But I don't specifically need them for my use case. |
This should be resolved using the latest v5.10.5 of ServiceStack on MyGet and upgrading servicestack Dart client to the latest ^1.0.27. |
Thanks for the fast turnaround. the .NET changes look good, and #721 is fixed on flutter app release builds, but unfortunately, neither issue is fixed on flutter web release. I get the same error. Making the same change in ListConverter: C# Codegen Version: 5.105 flutter doctor:
|
Then I'm not able to repro the issue. I'm running with the latest stable version of Flutter using their default app template:
Then add our integration test dtos which now includes both your
Which I'm able to call without issue, i.e: import 'package:servicestack/web_client.dart' if (dart.library.io) 'package:servicestack/client.dart';
IServiceClient client = ClientFactory.create("http://test.servicestack.net");
var result = await client.get(FooRequest());
var result = await client.get(RawBazRequest()); That I was able to test in release mode in Chrome with:
If you're not using the latest stable build please switch to that, otherwise I'll need a repro I can run locally in order to identify & resolve issues. |
Did you use/render the result of the API call? var result = await client.get(FooRequest());
print('result length: ${result.length}'); //this is fine. And if Foo had other properties, they would be ok to read here too.
print('result type (maybe minified): ${result.runtimeType} expected ${Foo().runtimeType}');
print('first bar type (maybe minified): ${result.first.runtimeType} expected: ${Bar().runtimeType}');
print('first bar.name property (crash!!): ${result.first.name}'); //crashes here when run on flutter web --release
print('first bar.description property (crash!!): ${result.first.description}'); |
Thanks! This fixes it. Tested both dev and release builds of web and Android, all issues fixed. It works on release web now even with |
Great, it's enabled by default because I believe resolving the minified type name from a reverse lookup is safer then relying on the heuristic of using the |
Say we have a API that returns a
Foo
:Rendering a list of
Bar
s showingname
anddescription
works fine in dev mode, but in release mode where minification is applied, trying to read thename
field throws:I spent a few hours digging and have the reason, and the fix:
It occurs due to reliance on
runtimeType
in SS's Json converters. We had previous issues with app obfuscation replacingList
's runtime type with _GrowableList. Web minification actually changes the runtimeType of all classes!Flutter advises not to ever use
runtimeType.toString()
here, and in my previous issue on this topic #721. I raised this issue with the Flutter team: dart-lang/language#1056 but the conclusion is the same: we can't use it.But! ServiceStack's json deserializer doesn't actually need to use
runtimeType
.Option 1:
Change
ListConverter
to use the passed-incontext.typeName
, rather than the runtime type:https://github.com/ServiceStack/servicestack-dart/blob/73df179e899cefe28765f3c14b934f5ab83ae63a/lib/json_converters.dart#L329
var genericArgs = getGenericArgs(getTypeName(o));
=>var genericArgs = getGenericArgs(context.typeName ?? getTypeName(o));
This works for my specific problem, but you'll probably need to do the same for MapConverter etc and run your unit tests. You could then probably completely remove this part https://github.com/ServiceStack/servicestack-dart/blob/73df179e899cefe28765f3c14b934f5ab83ae63a/lib/interfaces.dart#L69 where you add the runtime types of all generic types to
TypeContext.types
.Option 2:
Add the runtimeType of every dto class to the
TypeContext.types
map, not only generic classes:just delete this
where
line:https://github.com/ServiceStack/servicestack-dart/blob/73df179e899cefe28765f3c14b934f5ab83ae63a/lib/interfaces.dart#L69
...and a proper fix for #721
I admit I led you astray by suggesting the runtimeType of a
List<T>
was always "List"!The original problem was actually, for an API that returns a raw
List<Bar>
, there was no corresponding TypeToken included at the bottom of thedto.dart
, for the List, just one forBar
.The correct fix is for this to be included in the codegen server-side. But here's a workaround for
JsonWebClient
andJsonServiceClient
:https://github.com/ServiceStack/servicestack-dart/blob/73df179e899cefe28765f3c14b934f5ab83ae63a/lib/web_client.dart#L505
This workaround isn't ideal, as
The preferred fix is to include the List type in the generated dto server-side.
The text was updated successfully, but these errors were encountered: