shamblett / cbor

A CBOR implementation for Dart
MIT License
36 stars 16 forks source link

runtimeType String checking fails for web release builds #21

Closed thomasostfeld closed 3 years ago

thomasostfeld commented 3 years ago

The Encoder.writeArray function converts the elements' runtimeType to Strings to determine the correct encoding function for each one.

This fails on a release build of a Flutter web project due to dart2js and the minify process. A List<dynamic> gets turned into JSArray which implements List but doesn't contain that word in its type.

In addition to this, runtimeType can get minified. This seems to only occur for nested Lists. For example:

encoder.writeArray([1, 'str', ['test', 'str2']]);

The first two elements keep their runtimeType.toString() as 'int and 'String' but the third element becomes 'minified:u<String>'.

I tried fixing the JSArray problem with this:

if (valType.contains('JSArray')) {
  valType = 'List';
}

But this can't fix the minified type names. I guess the only solution would be proper type checking instead of unsafe String comparisons.

Please help me with this, I have to release my project soon and this can only be prevented by disabling every optimization of the default build process. This kills performance and blows up the size of the app by three times.

shamblett commented 3 years ago

The reason I used runtimeType(which I never liked BTW) was that the types don't come back as you would expect, for instance a type of 'List' that you declare say 'List fred = [];' actually has type of '_implUnorderedList' or something like that that Dart decides it is, it never seems to return 'List'.

I would like to get rid of the runtimeType stuff, I'll have another look at this, things have moved on in Dart since cbor was first written but I can't guarantee I can fix this. You may be better asking on the Dart SDK list also to see if they have any suggestions.

shamblett commented 3 years ago

OK, with guidance from the Google guys I've rid the package of the runtime type string conversions, the way to do this is to use the 'is' operator although its not as straightforward as you would think, there are a few gotcha's in the way this works. The unit test suite runs without error so as far as I can test its OK.

Package re published at 4.0.1, please see if this is any better for you.

thomasostfeld commented 3 years ago

After a quick test everything seems to work now. Thanks so much :)

I've had my fair share of problems with 'is' checks as well in the past, I can imagine what you mean.