Add Hermes support to React Native Code Push CLI

[eggli]

This is a respond PR to the request in React Native Code Push repository:

microsoft/react-native-code-push#1626

This will enable CodePush to support the latest JS to Byte Code engine, the Hermes from Facebook to boost up Android's performance.

It will check if Hermes is enabled in the android/app/build.gradle, and invoke Hermes to compile the JS bundle to a byte code bundle, replace the original JS bundle with the newly created byte code bundle, and finally, upload the byte code bundle to App Center.

For a further explanation, a Hermes enabled Android build is capable to execute both JS and Hermes byte code bundle, hence we don't have to modify any code in the client side(react-native-code-push).

Screenshot:

[msftclas]

All CLA requirements met.

[owenniblock]

Thanks for your contribution @eggli - let me discuss this with the Code Push team before we proceed with a code review.

I did a quick pass though and I'd love to see some unit tests in here (I think that's turning in to my mantra).

It would also be great if you can add some details to how this works to the PR (screenshots / terminal output etc).

[owenniblock]

The Code Push team love this - thanks @eggli - let me take a look at the code one more time and do a proper review - is there any change you could add some unit tests in the meantime please?

[owenniblock]

OK - loving this! Mainly minor stuff - if we can knock this in to shape I'll get it merged :)

[eggli]

Thanks for your contribution @eggli - let me discuss this with the Code Push team before we proceed with a code review.

I did a quick pass though and I'd love to see some unit tests in here (I think that's turning in to my mantra).

Yet I'm not seeing any existing test cases for release code push on react native here, would recommend that we do that in a separate PR to fill the test cases we lack.

It would also be great if you can add some details to how this works to the PR (screenshots / terminal output etc).

Yeah, sure, will do this.

[owenniblock]

would recommend that we do that in a separate PR to fill the test cases we lack.

Good idea. This all looks good to me - merging!

[willholen]

Note that we're in the process of migrating the NPM name from hermesvm to hermes-engine, so that'll need to be tweaked soon

[owenniblock]

Thanks for the heads-up @willholen :)

[amitse]

Does Apple App Store allow developers to perform these types of updates?
[Apple's Developer agreement)[https://developer.apple.com/programs/information) section 3.3.2?

[jstheoriginal]

@amitse

The Hermes engine is only applicable to Android. But if you're talking about CodePush updates in general:

3.3.2 Except as set forth in the next paragraph, an Application may not download or install executable code. Interpreted code may only be used in an Application if all scripts, code and interpreters are packaged in the Application and not downloaded. The only exceptions to the foregoing are scripts and code downloaded and run by Apple's built-in WebKit framework or JavascriptCore, provided that such scripts and code do not change the primary purpose of the Application by providing features or functionality that are inconsistent with the intended and advertised purpose of the Application as submitted to the App Store.

It's fine since React Native/CodePush uses Apple's JavascriptCore framework, and as long as you don't modify your app to the point that it's primary purpose becomes different than what you had submitted for the app review process.

[eXigentCoder]

Thanks for the great work!

I was trying to test it out on windows, using appcenter version 2.1.1 but i'm getting the following error:

I thought maybe that hermesvm needed to be installed globally so gave that a shot but still getting the error.

If I use the code-push@2.1.9 cli which I believe doesn't have hermes support everything works fine.

[jstheoriginal]

Yeah I’m getting that as well on 2.1.1 (but not using the hermes engine). I posted an issue for this here #634

[NicholasGWK]

Bump: Posted a possible solution in #634, can do a PR if that makes sense.

[owenniblock]

@NicholasGWK a PR would be the best way forward if you have it working. I'm sure this is working for some of my colleagues so it might be worth us figuring out why it's changed and maintaining backwards compatibility if that makes sense.

@eggli - any input from your perspective?

[eggli]

@owenniblock Sorry was in the middle of a relocation to foreign country, and, yeah, I was kinda confused in the parsing result form g2js when I was working on this, it's weird yet I thought it was an intended result. And I think it a great idea that we should try to figuring out why g2js is giving different results.

[StebanDev]

Thanks for the great work!

I was trying to test it out on windows, using appcenter version 2.1.1 but i'm getting the following error:

I thought maybe that hermesvm needed to be installed globally so gave that a shot but still getting the error.

If I use the code-push@2.1.9 cli which I believe doesn't have hermes support everything works fine.

I got the exact same error with 2.1.1 cli only when enabling hermes, I also tried the given solution by @NicholasGWK but then i got another error, it doesn't event get to this part:
Converting JS bundle to byte code via Hermes, running command:

[brerdem]

Same problem here. I can confirm it only happens in Windows 10. I did successfully released a bundle with a Mac @StebanDev @eggli

Thanks for the great work!
I was trying to test it out on windows, using appcenter version 2.1.1 but i'm getting the following error:

I thought maybe that hermesvm needed to be installed globally so gave that a shot but still getting the error.
If I use the code-push@2.1.9 cli which I believe doesn't have hermes support everything works fine.

I got the exact same error with 2.1.1 cli only when enabling hermes, I also tried the given solution by @NicholasGWK but then i got another error, it doesn't event get to this part:
Converting JS bundle to byte code via Hermes, running command:

[NicholasGWK]

Just to clarify the "solution" I posted is just to keep it from crashing when checking for the enableHermes flag. In my case I am not using hermes, so it goes ahead as usual. If there's problems after it checks the config, there might be a separate issue.

[StebanDev]

Same problem here. I can confirm it only happens in Windows 10. I did successfully released a bundle with a Mac @StebanDev @eggli

I can confirm it too, i released it successfully using Linux or WSL.

[owenniblock]

Please could someone create a fresh Issue for this? I'm worried that this conversation is going to get missed by the powers that be as it's on a Merged PR :)

If anyone with the issue on Windows fancies having a look and contributing - that would be amazing - otherwise I'll get an internal bug raised for triage against the right team if I can.

[StebanDev]

#638 If more information is needed, I will be glad to help.

[pierroo]

Note that we're in the process of migrating the NPM name from hermesvm to hermes-engine, so that'll need to be tweaked soon

Hello @willholen

Do you know how it is going with using hermes-engine instead of hermesvm?

I discovered this page wondering why my codepush suddenly didn't work since I moved to hermes with hermes-engine; but now even with the latest codepush-CLI and without getting any error when I release, it doesn't seem to be working as my app is not fetching my update. :(

Perhaps @StebanDev you know more about this as well?

I am truly in need of help, I am no master when it comes to these tools...

Thank you so much in advance!