-
-
Notifications
You must be signed in to change notification settings - Fork 4
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
Fix TIMOB-23198 - Hyperloop: Android can't access fields with primitive array types #16
Conversation
// Note lack of byte[], since bridge doesn't handle that, we treat it specially in wrap | ||
|| item instanceof int[] || item instanceof double[] | ||
|| item instanceof float[] || item instanceof short[] | ||
|| item instanceof long[] || item instanceof boolean[]; |
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.
Checked the java docs for primitive types. Do we need to list char
here as well?
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.
That's a good catch - but I'm not really sure how we should handle Character and char[].
The issue here is that:
- the bridge doesn't handle char or char[] conversions
- char is kind of unique in java in that it's essentially a 16-bit unsigned value meant to represent a Unicode character. So do we try and represent a char[] as String, or do we represent it as something more like short[] or int[]? (short is a 16-bit signed value, int is 32-bit signed) - to me the simple answer is turn char[] into String, but as single char into a String of length 1? Seems a little odd, but also I have seen some unique cases where people treat a char as a number more than a "character".
It looks to me like we carefully (or luckily?) avoided char[] and char altogether in terms of surfaced API.
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.
As another reference: NativeScript seems to treat char as a String: https://docs.nativescript.org/runtimes/android/marshalling/java-to-js.html
I'm going to guess char as a whole could likely use some loving here so that we:
- Can accept a JS string for a Character/char/char[] argument in a method call (or assigning to a Character/char/char[] field)
- Return a JS string when a field is of type Character/char/char[], or a method returns Character/char/char[]
Not sure if we also want to be even more liberal and accept JS Numbers (provided they are in a valid range) for char (or array of numbers for char[])
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.
The easier to use for the developers the better it is. Pedro once supported full conversation of array, numbers, objects etc. to native types, but Jeff removed it in his implementation. So we need to see how both platforms would do it to achieve as much parity as possible, even in these points.
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.
I'd propose we merge this for now, and open a JIRA ticket for handling char/char[]/Character separately.
Approved! |
Followup ticket: https://jira.appcelerator.org/browse/TIMOB-23213 |
https://jira.appcelerator.org/browse/TIMOB-23198