-
Notifications
You must be signed in to change notification settings - Fork 905
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
refactor: init now expects templates to have react-native version set #2422
Conversation
@blakef let's make the CI green? 👍 |
dc419de
to
d709c04
Compare
d709c04
to
d55f6f9
Compare
d55f6f9
to
315dc0a
Compare
I've made the changes requested and tested against:
The depends on react-native-community/template#30 having published artefacts. |
) There was a discussion on Discord, where we looked at implementing what was originally proposed. This is that implementation. It'll need to land with a [cli](react-native-community/cli#2422) change to use the update template + tagging. - feat: add mutating react-native & @react-native/ packages Support for doing this on nightly and release workflows - feat: added node ./scripts/updateReactNativeVersion.js <version> Sets the version of react-native and the @react-native/ scoped packages. It will also normalize tags to concrete versions. E.g. latest -> 0.74.2 - refactor: move scripts into a ./scripts folder
) There was a discussion on Discord, where we looked at implementing what was originally proposed. This is that implementation. It'll need to land with a [cli](react-native-community/cli#2422) change to use the update template + tagging. - feat: add mutating react-native & @react-native/ packages Support for doing this on nightly and release workflows - feat: added node ./scripts/updateReactNativeVersion.js <version> Sets the version of react-native and the @react-native/ scoped packages. It will also normalize tags to concrete versions. E.g. latest -> 0.74.2 - refactor: move scripts into a ./scripts folder
// this applies to all version before 0.75. The 1st release candidate is the minimal | ||
// version that has no template. | ||
// Does the react-native@version package *not* have a template embedded. We know that this applies to | ||
// all version before 0.75. The 1st release candidate is the minimal version that has no template. | ||
const useLegacyTemplate = semver.lt( | ||
version, | ||
TEMPLATE_COMMUNITY_REACT_NATIVE_VERSION, |
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 0.75.0-rc.0
so I'm unsure how this will work out with lt
.
Should we just instead compare the minor from semver and do <= 74
?
// Suppose react-native 0.75.0 was released, and 0.74.8 was the previous version. If I ran | ||
// `init --version 0.74.8` it would be guaranteed to work with the latest version of the template | ||
// matching the MAJOR.MINOR, e.g. 0.74.21. | ||
return `${TEMPLATE_PACKAGE_COMMUNITY}@~${version}`; |
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.
Here you should recompose the version no?
As what's going to happen if version is 0.75.0-rc.0
?
What will ~0.75.0-rc.0
resolve? It should be ~0.75.0
instead.
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'm adding a branch for nightly
which is the only tag that doesn't play nicely with ~
.
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've made the changes requested and tested against:
Test | Outcome | Notes |
---|---|---|
node ./packages/cli/build/bin.js init TestInit --skip-install --version nightly |
✅ | |
node ./packages/cli/build/bin.js init TestInit --skip-install --version next |
❌ but expected | Uses the 0.75.0-rc.0 version of the template which has the node modules issue |
node ./packages/cli/build/bin.js init TestInit --skip-install --version 0.75.0-rc.0 |
❌ but expected | Uses the 0.75.0-rc.0 version of the template which has the node modules issue |
node ./packages/cli/build/bin.js init TestInit --skip-install --version next --template @react-native-community/[email protected] |
✅ | It won't build, but the project generates correctly - React Native is 1000.0.0 |
node ./packages/cli/build/bin.js init TestInit --skip-install --template @react-native-community/[email protected] |
✅ | It won't build, but the project generates correctly - React Native is 1000.0.0 |
The depends on react-native-community/template#30 having published artefacts.
For 0.75's previous RCs, we didn't have the infrastructure in place to tag @react-native-community/templates 0.75-stable branch correctly, as well as publishing the corresponding version to npm. For example: If @react-native-community/[email protected] is the latest version published, and I specified --version next: I'd expect 0.75.1 of the template to be used with 0.75.0-rc.1 of react-native. The downside of this approach is, if you init an older version then things might not work. You'd have to specify the matching template using --template @react-native-community/template@<some version>. Given that init is intended for new projects this isn't a concern for most users. We could look at abusing the `scripts` field to expose the corresponding version of react native in the npm registry: https://registry.npmjs.org/@react-native-community/template Something like: "scripts": { "version": "0.75.1-rc.2" }
Useful to see the error message instead of swallowing it if the user enables `--verbose`.
315dc0a
to
9ac0e05
Compare
Summary:
This will take advantage of changes coming in the way we publish the template: react-native-community/template/pull/30
Test Plan:
TODO: Run each of the combinations of the update table in the code. Might be difficult to do with unpublished @react-native-community/template npm packages published.
Checklist