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

fix(cli): initialising in projects using Yarn v3 #2025

Merged
merged 1 commit into from
Jan 24, 2024

Conversation

szymonrybczak
Copy link

Please select one of the following

  • I am removing an existing difference between facebook/react-native and microsoft/react-native-macos 👍
  • I am cherry-picking a change from Facebook's react-native into microsoft/react-native-macos 👍
  • I am making a fix / change for the macOS implementation of react-native
  • I am making a change required for Microsoft usage of react-native

Summary:

Recently in React Native Community CLI we added bumping Yarn version in every new project created to Yarn v3 (PR for more context). And In Yarn v3 flag -s doesn't exists anymore, so the react-native-macos-init tool won't work.
CleanShot 2024-01-09 at 17 29 31
I've replaced usage of -s with --silent flag. Also this change is backward compatible, since Yarn v1 support --silent flag.

CleanShot 2024-01-09 at 17 29 47

Changelog:

[GENERAL] [FIXED] - Initialising react-native-macos in project using Yarn v3 with react-native-macos-init

Test Plan:

  1. Create new project with npx react-native@latest init and bump Yarn version to 3 (we didn't released changes with bumping version by default yet.
  2. Try running react-native-macos-init

@szymonrybczak szymonrybczak requested a review from a team as a code owner January 9, 2024 16:55
@szymonrybczak
Copy link
Author

cc @Saadnajmi @tido64

@Saadnajmi
Copy link
Collaborator

cc @Saadnajmi @tido64

I was actually thinking of moving the repo to yarn v4 (I guess 3 for the template project works though). Do either of you see any issue with that?

@szymonrybczak
Copy link
Author

Do either of you see any issue with that?

Just make sure to properly set nodeLinker value in the Yarn config file, I think that's the most important thing.

Copy link
Collaborator

@Saadnajmi Saadnajmi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved, might take a bit to merge as we have some CI issues to fix.

@Saadnajmi
Copy link
Collaborator

So this PR is ready to go, but to release a new version of react-native-macos-init, we use beachball, which works by generating a change file (aka, running yarn change). For some reason, that is not working, so if I merge this as is, we won't get a new package. Bear with me while I sort that out!

@Saadnajmi
Copy link
Collaborator

@szymonrybczak Could you run yarn change --type patch --message 'fix(cli): initialising in projects using Yarn v3' in packages/react-native-macos-init to hopefully force a change file to show up and get added as a commit?

@szymonrybczak
Copy link
Author

hey @Saadnajmi, so the command that you sent is not working with current setup. I checked why, and it's because the root package.json has:

"beachball": {
"shouldPublish": false
},
and packages/react-native-macos-init has the same value, so: beachball.shouldPublish=false. After tweaking this value in packages/react-native-macos-init/package.json to true the change file is correctly added and generated.

diff --git a/change/react-native-macos-init-cf2af331-a6c9-40dd-a2f2-f599609799bd.json b/change/react-native-macos-init-cf2af331-a6c9-40dd-a2f2-f599609799bd.json
new file mode 100644
index 0000000000..1ee6fb29f3
--- /dev/null
+++ b/change/react-native-macos-init-cf2af331-a6c9-40dd-a2f2-f599609799bd.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "fix(cli): initialising in projects using Yarn v3",
+  "packageName": "react-native-macos-init",
+  "email": "szymon.rybczak@gmail.com",
+  "dependentChangeType": "patch"
+}

Do you think we can change this value for this package? and should we do it in under this PR or in the separate?

@Saadnajmi
Copy link
Collaborator

hey @Saadnajmi, so the command that you sent is not working with current setup. I checked why, and it's because the root package.json has:

"beachball": {
"shouldPublish": false
},

and packages/react-native-macos-init has the same value, so: beachball.shouldPublish=false. After tweaking this value in packages/react-native-macos-init/package.json to true the change file is correctly added and generated.

diff --git a/change/react-native-macos-init-cf2af331-a6c9-40dd-a2f2-f599609799bd.json b/change/react-native-macos-init-cf2af331-a6c9-40dd-a2f2-f599609799bd.json
new file mode 100644
index 0000000000..1ee6fb29f3
--- /dev/null
+++ b/change/react-native-macos-init-cf2af331-a6c9-40dd-a2f2-f599609799bd.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "fix(cli): initialising in projects using Yarn v3",
+  "packageName": "react-native-macos-init",
+  "email": "szymon.rybczak@gmail.com",
+  "dependentChangeType": "patch"
+}

Do you think we can change this value for this package? and should we do it in under this PR or in the separate?

derp, I missed that in the react-native-macos-init package.json. Yes, please set it to true in this PR and then I'll merge :)

@szymonrybczak
Copy link
Author

derp, I missed that in the react-native-macos-init package.json.

The thing is that it wasn't specified! I assume that is getting value from the root's package.json 😅

@szymonrybczak
Copy link
Author

@microsoft-github-policy-service agree company="Callstack"

@szymonrybczak
Copy link
Author

@microsoft-github-policy-service agree

@Saadnajmi
Copy link
Collaborator

Saadnajmi commented Jan 23, 2024

@szymonrybczak Looks like there are other issues I'll still have to investigate. We haven't had to release a new version of react-native-macos-init since it was created really, so I'm realizing now the CI to publish stuff as issues 😅. I'll try to make that fix and then update back here. Meanwhile, I think you can undo your last commit and I'll merge the PR as is.

@szymonrybczak
Copy link
Author

@Saadnajmi ah, okay. I've removed the last commit. Should be good to merge.

@Saadnajmi
Copy link
Collaborator

@Saadnajmi ah, okay. I've removed the last commit. Should be good to merge.

Sorry, last two commits. Looks like adding the change file is what causes the break

@Saadnajmi Saadnajmi merged commit b15e168 into microsoft:main Jan 24, 2024
18 checks passed
@seanstrom
Copy link

Has the the NPM package been updated for this script?
If not could someone publish an update please 🙏

cc @szymonrybczak @Saadnajmi

@Saadnajmi
Copy link
Collaborator

@seanstrom I think not, I can look into it

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.

4 participants