Skip to content
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

Inconsistent ObjectInstance.GetOwnProperties #1945

Closed
lofcz opened this issue Aug 16, 2024 · 4 comments
Closed

Inconsistent ObjectInstance.GetOwnProperties #1945

lofcz opened this issue Aug 16, 2024 · 4 comments

Comments

@lofcz
Copy link
Contributor

lofcz commented Aug 16, 2024

Version used

4.0.1

Describe the bug

I'm working on a Blazor debugger for Jint. Recently I've encountered a weird inconsistency - if the thing is intentional, feel free to yell at me. ObjectInstance.GetOwnProperties() behaves (at least from my perspective) differently for CLR ObjectWrapper and native Object instances.

For CLR objects, only properties and fields get reported:

// we take public properties and fields
foreach (var p in ClrType.GetProperties(BindingFlags.Static | BindingFlags.Instance | BindingFlags.Public))
{
    var indexParameters = p.GetIndexParameters();
    if (indexParameters.Length == 0)
    {
        var jsString = JsString.Create(p.Name);
        yield return jsString;
    }
}

foreach (var f in ClrType.GetFields(BindingFlags.Static | BindingFlags.Instance | BindingFlags.Public))
{
    var jsString = JsString.Create(f.Name);
    yield return jsString;
}

While native objects report properties, which are methods as well. For example:

var myTest = {
  myFn: () => { 
    return 4
  }
}

reports myFn when calling GetOwnProperties on a corresponding JsValue, while only props and fields are reported for CLR objects as per the code above.

While I understand that myFn is a property from the JS perspective and public void MyFn() is a method from CLR perspective, it still seems inconsistent, given we report fields from CLR objects already.
My understanding is that GetOwnProperties is the tool to check from the CLR side, how the JS interpreter sees the given object, hence my suggestion to include methods too.

Expected behavior

Methods of CLR objects are reported from GetOwnProperties (we include GetMethods result in the code above).

@lahma
Copy link
Collaborator

lahma commented Aug 16, 2024

Adding public methods sounds reasonable as they can be invoked. Would you like to create a PR with some test coverage?

@lofcz
Copy link
Contributor Author

lofcz commented Aug 16, 2024

Sure, I'll go ahead. Technically, I would consider this breaking, since consumers might be relying on the fact that the method in question works the way it does now, so this might warrant a 4.1 release.

@lahma
Copy link
Collaborator

lahma commented Aug 16, 2024

One option is to add new enum flags property to interop options defining returned properties and omit methods for now.

@lofcz
Copy link
Contributor Author

lofcz commented Aug 17, 2024

solved in #1947

@lofcz lofcz closed this as completed Aug 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants