-
Notifications
You must be signed in to change notification settings - Fork 128
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
RFC: lean core JSC #836
base: main
Are you sure you want to change the base?
RFC: lean core JSC #836
Conversation
Is that 32mb of space saved on app bundles downloaded by end users? |
no, it's only on the developer's building host. |
Nice, still awesome to see! Love this. |
FYI @Saadnajmi |
|
||
## Proposed Changes and Timeline | ||
|
||
- Version 0.77: Remove the [`jsc-android` npm dependency](https://github.com/facebook/react-native/blob/2d337efc23c662143b3f39a6c994d80fec047054/packages/react-native/package.json#L132) from React Native and instead download the artifact from Maven Central. When using Hermes, downloading the unused npm dependency is unnecessary and removing it saves about 32 MB of uncompressed size. |
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.
Is this needed in 0.77 for 16KB page support?
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.
yes, but if there's concern to cherry-pick facebook/react-native#47972 to 0.77. we can postpone the mavenCentral migration to 0.78. on 0.77 we still use jsc-android npm package for a newer version with android 16kb page support.
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'd rather go with 0.78.
Our take on 16K support is that it's Hermes first. Originally we were not even planning to include JSC for 16K support.
|
||
## Adoption strategy | ||
|
||
We will offer a drop-in replacement with the `react-native-javascriptcore` npm package while maintaining JSC support in the core for two additional React Native releases to ensure a smooth transition. |
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.
If the boundary is jsi::Runtime, then I think that means there could be frequent updates needed against RN package.
The current structure has a nice benefit of keeping it up to date automatically, since it must build in line with JSI changes. But I think it means the project would need a long term maintainer to continue working. See microsoft/v8-jsi@4438466
Depending on whether there is a natural maintainer, the other option would look like a harder deprecation, which would still allow externally maintained runtimes to develop.
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.
this is also the same problem for https://github.com/Kudo/react-native-v8. i can try to maintain the in my spare time (also love to have other maintainers from community)
maybe we need to include nightly testing from v8 and react-native-javascriptcore, so that we can catch jsi interface breaking changes and take action earlier.
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.
We'd be happy to help with maintenance of that RN <-> JSC integration at Callstack
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.
awesome! love the collaboration with someone from Callstack then.
This align with our plans for next year. Not sure whether we can do it in 0.77, because the RC0 is already out and we want to reduce the stabilization time. We might have to shift the timeline by 1 release, but next year we plan to do a release every couple of months, so it shouldn’t delay it that much. @cortinico I know you have thoughts on this! 😄 |
that's fair. we can also do npm |
|
||
Since Hermes has become the dominant JavaScript engine in the React Native ecosystem, it is inefficient to include unused JavaScriptCore code and binaries in the core. By extracting JavaScriptCore to a separate, third-party maintained library, we can streamline the core and reduce unnecessary bloat. | ||
|
||
## Adoption strategy |
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.
Q: How will this work? Is it going to work through Community CLI autolinking? If so, we'll have to extend it so that when react-native-javascriptcore
is added, the hermes dependency is excluded.
Could you add a paragraph about it?
- Version 0.78 or 0.79: Introduce a new `react-native-javascriptcore` npm package with a newer version of JavaScriptCore for Android. In this version, we will only provide the [Intl variant](https://github.com/react-native-community/jsc-android-buildscripts/blob/9c61fece4753902a2cd6d29dfa46b7b521f0c821/README.md#international-variant), as newer JavaScriptCore versions do not support disabling Intl. The existing JSCRuntime from the core should still function with this version but will display a deprecation warning during build time. A challenge will be managing the core JSCRuntime alongside the third-party JSCRuntime from `react-native-javascriptcore`; further study is required. | ||
- Version 0.80: Completely remove JavaScriptCore from the core: | ||
- Eliminate `JSCRuntime` code. | ||
- Deprecate Gradle's `hermesEnabled` and Podfile's `hermes_enabled`, making them no-ops. |
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.
This has hidden complexity. Check here:
https://github.com/search?type=code&q=repo%3Afacebook%2Freact-native+%22hermesEnabled%22+language%3AKotlin&l=Kotlin
If we deprecate hermesEnabled
and set it to true
, we'll always produce a bundle that is not processed by hermesc
which jsc won't be able to consume.
So in a sense we need to refactor the build logic on Android & iOS so that you can still decide if hermesc
should execute or not.
Moreover on Android, we selectively remove .so
based on the hermesEnabled
so that logic will have to be adopted as well.
|
||
## Proposed Changes and Timeline | ||
|
||
- Version 0.77: Remove the [`jsc-android` npm dependency](https://github.com/facebook/react-native/blob/2d337efc23c662143b3f39a6c994d80fec047054/packages/react-native/package.json#L132) from React Native and instead download the artifact from Maven Central. When using Hermes, downloading the unused npm dependency is unnecessary and removing it saves about 32 MB of uncompressed size. |
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'd rather go with 0.78.
Our take on 16K support is that it's Hermes first. Originally we were not even planning to include JSC for 16K support.
- Eliminate `JSCRuntime` code. | ||
- Deprecate Gradle's `hermesEnabled` and Podfile's `hermes_enabled`, making them no-ops. | ||
- Update documentation at https://reactnative.dev/docs/hermes. | ||
|
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.
Can we add a paragraph mentioning that we'll stop testing JSC in the release testing matrix (you can link this page: https://github.com/reactwg/react-native-releases/blob/main/docs/guide-release-testing.md)
writing a proposal to move JSC away from core
View the rendered RFC