Skip to content

chore: Refactor JSI conversion#798

Open
msluszniak wants to merge 5 commits intomainfrom
@ms/refactor-jsi-conversion
Open

chore: Refactor JSI conversion#798
msluszniak wants to merge 5 commits intomainfrom
@ms/refactor-jsi-conversion

Conversation

@msluszniak
Copy link
Member

Description

Introduces a breaking change?

  • Yes
  • No

Type of change

  • Bug fix (change which fixes an issue)
  • New feature (change which adds functionality)
  • Documentation update (improves or adds clarity to existing documentation)
  • Other (chores, tests, code style improvements etc.)

Tested on

  • iOS
  • Android

Testing instructions

  • Run tests and check that the results are the same
  • Run all demo apps and check that there are no degradations in any functionality

Screenshots

Related issues

Closes #785

Checklist

  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have updated the documentation accordingly (No changes in public API)
  • My changes generate no new warnings

Additional notes

@msluszniak msluszniak self-assigned this Feb 10, 2026
Copy link
Collaborator

@mkopcins mkopcins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

approved by mistake

@msluszniak msluszniak changed the title Refactor JSI conversion chore: Refactor JSI conversion Feb 11, 2026
@msluszniak msluszniak requested a review from mkopcins February 11, 2026 20:57
Copy link
Collaborator

@chmjkb chmjkb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than that I think it's good

Comment on lines 56 to 58
if (!isValidTypedArray) {
throw jsi::JSError(runtime, "Value must be a TypedArray");
throw jsi::JSError(runtime, "Value must be an ArrayBuffer or TypedArray");
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if we should throw the rnexecutorch error here

Comment on lines -340 to -346
inline jsi::Value getJsiValue(const std::vector<char> &vec,
jsi::Runtime &runtime) {
jsi::Array array(runtime, vec.size());
for (size_t i = 0; i < vec.size(); i++) {
array.setValueAtIndex(runtime, i, jsi::Value(vec[i]));
}
return {runtime, array};
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we deleting this because it's not used? Or there's some other reason

jsi::String::createFromUtf8(runtime, vec[i]));
}
return {runtime, array};
inline jsi::Value getJsiValue(int val, jsi::Runtime & /*runtime*/) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: Same for the rest of functions

Suggested change
inline jsi::Value getJsiValue(int val, jsi::Runtime & /*runtime*/) {
inline jsi::Value getJsiValue(int32_t val, jsi::Runtime & /*runtime*/) {

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Create a template function for translating std::vector<T> to jsi::value in JSIConversion.h

3 participants