Skip to content
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

Update package.json for 1.17.3-beta and add peerDependencies #761

Merged
merged 2 commits into from
Mar 22, 2017

Conversation

richardhuaaa
Copy link
Contributor

As we are about to publish a breaking change for RN 0.43, it might be good to start indicating our peerDependencies clearly so that users don't install mismatching RN and CodePush versions. Peer dependencies are documented here: https://nodejs.org/en/blog/npm/peer-dependencies/.

Note this range won't match -rc releases, but I think it should be okay in this case because all of the versions in this peerDependencies list have non-rc versions that people should be using.

@sergey-akhalkov, @max-mironov, what do you think?

@max-mironov
Copy link
Contributor

@Silhouettes do I get it right that RN v0.19 will be ok and fully compatible with latest release of our plugin?

@sergey-akhalkov
Copy link
Contributor

sergey-akhalkov commented Mar 22, 2017

@Silhouettes, looks good to me, but do I get it right it probably means that we have to update peerDependencies each time new version of RN is released:

  1. release react-native-code-push@1.18.0 due to breaking changes in RN 0.43 and update peer dependencies to peerDependencies: "0.43.x"
  2. release react-native-code-push@1.18.1 due to RN 0.44 has been release without any breaking changes, update peer dependencies to peerDependencies: "0.43.x-0.44.x"
  3. release react-native-code-push@1.19.0 due to breaking changes in RN 0.45 and update peer dependencies to peerDependencies: "0.45.x"
  4. and so on... ?

@nihgwu
Copy link
Contributor

nihgwu commented Mar 22, 2017

@sergey-akhalkov no need to update peerDependencies each time RN releases new version

  1. peerDependencies: ">= 0.43.0"
  2. peerDependencies: ">= 0.43.0"
  3. peerDependencies: ">= 0.45.0"

@richardhuaaa
Copy link
Contributor Author

Thanks for the feedback. I've thought a bit more about the problem I am trying to solve here, and I no longer think we should do this. Basically, there are two types of users, and they should be both taken care of if we follow the semver standard and publish the breaking change as react-native-code-push@2.0.0-beta when RN 0.43 comes out:

  1. Existing users. They will be on CodePush 1.x and RN <= 0.42, which is a working setup. They won't upgrade to 2.0.0-beta (a major version increment!) without investigating whether it is compatible with their setup. They might upgrade to RN 0.43, but if CodePush breaks there, then they will instinctively look for the latest release of CodePush (2.0.0-beta), which will work.
  2. New users - by default, they will set up with RN 0.43 (latest) and CodePush 2.0.0-beta (latest), and that will work for them. They won't install CodePush 1.x or RN < 0.43 without good reason.

Basically, we just need to uphold two invariants - that breaking changes in CodePush will be represented by a major version bump, and that the latest React Native version will always be supported by the latest CodePush version.

Given that the above should just work, I think it makes less sense to tolerate the issues that might be introduced by using peerDependencies:

  1. Using an expression like >= 0.43 may give the false impression that we support all subsequent RN versions. For example, if RN 0.45 is a breaking change, then a small subset of users might try to upgrade to it with the assumption that CodePush will just work, and find that our peerDependencies section confuses them more than it helps them. The alternative to using >= 0.43 is to update the expression on every release, like @sergey-akhalkov mentions.
  2. An expression like >= 0.43.0 does not encompass rc versions, for example 0.43.0-rc. An expression like >= 0.43.0-0 will match 0.43.0-rc, but it won't match rc versions on later releases, for example 0.44.0-rc. This is documented in Version comparators exclude rc releases npm/npm#8854. I had considered doing an expression like >= 0.43.0-0 || >= 0.44.0-0 || >= 0.45.0-0 to mimic what we want >= 0.43.0 to do, but given the reflection in the first half of this comment I'm now convinced that the added complexity is not worth it.

If people have more thoughts here, I'm open to hear them! However, I need to publish 1.17.3-beta today, so in the absence of further comments I will publish this version without the peerDependencies.

@richardhuaaa
Copy link
Contributor Author

Also in answer to your question @max-mironov - according to our compatibility table, this version should support 0.19. I'm not sure whether that is true or not as we haven't tested that version in a long time, but if it is not true, then I'm not suggesting that we should support that version - we would just update the compatibility table in that case.

@richardhuaaa richardhuaaa merged commit 212f222 into master Mar 22, 2017
@richardhuaaa richardhuaaa deleted the update-package-json branch March 22, 2017 21:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants