Remove overriden createJSModules

[ptomasroos]

Since of recently in master the unused createJSModules has been removed and ReactPackage's no longer contain the definition of createJSModules. There for it must be removed in order to not yield a compile error. This is breaking change.

facebook/react-native@ce6fb33

[msftclas]

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. We will now review your pull request.
Thanks,
Microsoft Pull Request Bot

[sergey-akhalkov]

Hi @ptomasroos, thank you for the contribution! LGTM, we'll merge it just before releasing new version of react-native-code-push

[janicduplessis]

@sergey-akhalkov Could this be merged soon? 0.47.0-rc is now released and this is needed for it to work.

[sergey-akhalkov]

Hi @janicduplessis, thanks for reaching us. We are following the approach when our master branch is always compatible with the latest stable version of RN (the current stable version is 0.46.4). So we are going to merge this PR just after RN 0.47 will be released, then we'll make sure if new version of RN does not break us, then we'll release new version of react-native-code-push that will support RN 0.47.

[patw0929]

RN 0.47.0 was released today, please merge this PR.
Thank you :)

[sergey-akhalkov]

Hi @patw0929, sure! We are going to test the PR with RN 0.47.0 today and merge it if everything is ok.

[sergey-akhalkov]

Hey @ptomasroos, @janicduplessis, @patw0929, the PR has been verified and merged!

[maxkomarychev]

@sergey-akhalkov which version of react-native-code-push should we install to have this patch included?

[max-mironov]

@maxkomarychev currently you can update from Master branch and we are going to release new version with this stuff in the nearest time!

[sergey-akhalkov]

Hey guys, we've released [email protected] - please let us know if you see any issues.

[jjdp]

is there no backwards compatibility < 0.47?
can't you just delete the @Override instead of the method

[max-mironov]

@jjdp sure that's a kind of solution but from our point of view that looks a little... hacky?
I mean here that we should keep step with what react-native do (and BTW that is what react-native officially suggest to do: facebook/react-native@ce6fb33)

To upgrade, remove all overrides of createJSModules and keeping calling your JS modules as before

That way we have released new update with incremented major version which means that other users (who use RN < 0.47) should not be affected.

More than that - that new release (5.0.0) do nothing but only add compatibility with latest react-native. So you should not worry about this release at all if you are on RN < 47 and there should be no issues with this release if you upgrade to RN47 and higher later.

[jjdp]

ah that makes sense

[cooperka]

@max-mironov so far as I can tell, there's actually nothing in that commit that "officially recommends" removing the entire method from external libraries. The recommendation is

To upgrade, remove all overrides of createJSModules

By removing the @Override annotation, that successfully resolves the error in a backwards compatible manner. It's not a hack, just good practice. Then people using older versions of React Native for whatever reason will still be able to receive updates to this library.

Here's a clean example of how to do it while preserving backwards-compatibility: https://github.com/rebeccahughes/react-native-device-info/pull/191/files

I understand that people on [email protected] won't be affected by this in the short-term, but in the long-term they will not be able to receive new features and bug fixes unless you manually release duplicate fixes to the older branch. It's more work for everyone.

[sergey-akhalkov]

Hi @cooperka, agreed, thanks for voicing your concerns, that really makes sense for users to restore backwards compatibility for RN 0.46.x

So we are going to do the following - we'll wait for bunch of fixed bugs or implemented features and then we'll release new version of react-native-code-push (saying 5.1.0 or 5.0.1) that will also include restoring compatibility with RN 0.46.x
Also, there is no need to support [email protected] due to 5.x.x will restore compatibility with RN v0.46, so users just will need to update their version of react-native-code-push (they will do it anyway if they want to receive bug fixes or new features for [email protected]), does it makes sense?

Please let me know if you have any other thoughts or considerations.

[cooperka]

Thanks for the reply @sergey-akhalkov, I think that sounds good! Cheers 🍻