[TIMOB-23990] Android: Improve TypeConverter::jsValueToJavaObject() implementation
GitHub Issue | n/a |
---|---|
Type | Improvement |
Priority | Medium |
Status | Open |
Resolution | Unresolved |
Affected Version/s | n/a |
Fix Version/s | n/a |
Components | Android |
Labels | n/a |
Reporter | Gary Mathews |
Assignee | Gary Mathews |
Created | 2016-10-06T11:38:09.000+0000 |
Updated | 2018-02-26T19:36:02.000+0000 |
Description
Quote from [~cwilliams] [GitHub](https://github.com/appcelerator/titanium_mobile/pull/8459#issuecomment-251833180)
{quote}
so clearly I think our existing behavior is basically flukey and happened by chance/luck more than anything. We should definitely revisit the behavior for jsValueToJavaObject, since order of comparisons clearly matters. The test failures on this past PR build should be pretty informative to see exactly what case broke by moving to a more explicit Number check. But I'd argue we should define the actual conversion order in a more formal way, likely trying explicit checks like IsNumberObject(), IsBooleanObject(), etc first, and if all of those fail, then moving down to the more forgiving checks (but again, what order we try those will still matter - not sure if there's any spec on that).
Looking at the code, it looks like this conversion is used when:
- we are trying to convert to a generic java.lang.Object
- we are trying to convert to a KrollProxy
- converting elements in varargs array
- the result of invoking a V8Function
- the result of calling a native property on a V8Object
- the result of nativeEvalString in V8Runtime
- converting js args into an Object[] (for each element)
- converting js array to java Object[] (for each element)
- converting keys and values in a JS Object (which this PR would fix to only be used for values)
- in jsObjectIndexPropsToJavaArray (which seems to be used only in some special edge case on creating a proxy object?)
I'm not sure if we can move away from using it so much and use more specific conversions where we know the target types in advance?
On another tangent, since we've updated V8 to ES6 compat, we should actually consider the new JS types that have been introduced and how to convert/handle those:
- Generators
- Promises
- Maps
- Sets
- Typed arrays (uint8, uint8 clamped, int8, uint16, int16, uint32, int32, float32, float64)
And I'm not sure we ever handled regexps. God knows how we'd handle those (we could convert to equivalent Patterns or something, but not sure how we'd handle state when doing matches or whatever).
{quote}
No comments