fix undefined value not converted to gobject #334
Conversation
|
Added unit test, failing on |
CapitaineJSparrow
force-pushed
the
fix/undefined-value
branch
from
e70b3d6
to
954b4ad
Compare
CapitaineJSparrow
force-pushed
the
fix/undefined-value
branch
from
954b4ad
to
cb9e667
Compare
| auto maybeDetailString = Nan::ToDetailString(value); | ||
| Nan::Utf8String utf8String( | ||
| !maybeDetailString.IsEmpty() ? | ||
| maybeDetailString.ToLocalChecked() : | ||
| UTF8("[invalid value]") | ||
| ); | ||
|
|
||
| // void/null | ||
| if (G_VALUE_TYPE(gvalue) == G_TYPE_INVALID) | ||
| if (G_VALUE_TYPE(gvalue) == G_TYPE_INVALID || strcmp(*utf8String,"undefined") == 0) | ||
| return value->IsNullOrUndefined(); |
There was a problem hiding this comment.
This check seems a bit more complex than it needs to be. The last line here already tells you how to check if a value is undefined, but you could only check for undefined with value->IsUndefined(). Which is way better because this function is called very often and we don't want a string comparisons to be run for each call to this function.
Could you also tell me a bit more about the root cause of #333? Reason is, this function checks if we can convert a V8 value into a GValue's type. But if we add a check if (... || value->IsUndefined()) return value->IsNullOrUndefined(), then we're returning true for any call to this where value === undefined, even if the GType we want to convert to isn't void.
There was a problem hiding this comment.
I'm sorry I don't have enough knowledge to tell you the root cause of #333 Consider my PR as a bug report I don't understand a lot about V8 and how the library works internally, neither cpp code. The only interesting part in my PR is the unit test. I'm marking this PR as draft.
But if we add a check if (... || value->IsUndefined()) return value->IsNullOrUndefined(), then we're returning true for any call to this where value === undefined, even if the GType we want to convert to isn't void.
You are completely right here, do you see a better approach to manage this case ?
Fix #333 (confirmed on my end)
I'm not sure about myself, undefined is not in glib constants so I compare the type of value as string. I'm new to this project and it's the first lines of C code I write since decades so may be not the right fix.