-
Notifications
You must be signed in to change notification settings - Fork 37
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
Adding some tests for checking the spreading behaviour of objects returned by a worklet. #180
base: main
Are you sure you want to change the base?
Conversation
example/Tests/worklet-tests.ts
Outdated
return ExpectValue(spreadObject, { a: 100 }); | ||
}, | ||
check_jsi_object_is_assignable_after_worklet: async () => { | ||
const fw = () => { |
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.
Please use better naming than two characters - something like func
here
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.
Changed
example/Tests/worklet-tests.ts
Outdated
"worklet"; | ||
return { a: 100 }; | ||
}; | ||
let wf = Worklets.defaultContext.createRunAsync(fw); |
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.
Also we can just do runAsync(func)
instead of the create stuff. It's less code.
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.
Changed it so
}; | ||
let wf = Worklets.defaultContext.createRunAsync(fw); | ||
const result = await wf(); | ||
const assignedObject = Object.assign({}, result); |
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 thought the problem was when using Object.assign/spread inside a Worklet?
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.
No, I have added a test below to show that spreading inside the worklet works as expected.
My use case was to spread the object that is returned by the worklet, and I have not spread inside worklet in production code, so I have no further experience with it.
The added
check_jsi_object_is_returned_from_worklet
test passes to show that the correct object is returned. It might be redundant compared the current main, I have not done an exhaustive check there.However, in contrast - but on top of the previous test - the two newly added tests
check_jsi_object_is_spreadable_after_worklet
andcheck_jsi_object_is_assignable_after_worklet
fail, showing that neither spread syntax norObject.assign()
can be used with the returned "object".