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

[RNTester] Correctly specify React Native dependency #2065

Merged
merged 1 commit into from
Feb 2, 2024

Conversation

Saadnajmi
Copy link
Collaborator

@Saadnajmi Saadnajmi commented Jan 31, 2024

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:

After #2030 landed, running yarn start (an alias of react-native start) in packages/rn-tester leads to this error:

command not found: react-native

This seems to be because Yarn 4 is much stricter about only using declared dependencies as compared to Yarn 1, and RNTester's package.json only specifies react-native as a peer dependency (a "*" at that!):

...
  "peerDependencies": {
    "react": "18.2.0",
    "react-native": "*"
  },
...

What we want is tell RNTester ( a private package that will never be published) to use the workspace copy of react-native, rather than download from NPM. Luckily, modern package managers have the workspace:* syntax for exactly that! Let's update our package.json. I also had to specify react-native-macos as the dependency instead of react-native.

This fix won't work upstream in React Native, where yarn 1 doesn't support workspace:*, and the version needs to update for every nightly / stable release. I have prepared a separate fix there: facebook#42756

Changelog:

[INTERNAL] [FIXED] - Fix running react-native start in rn-tester with yarn 4

Test Plan:

CI should pass.

@Saadnajmi Saadnajmi requested a review from a team as a code owner January 31, 2024 00:24
@Saadnajmi Saadnajmi changed the title [RN-Tester] Correctly specify React Native dependency [RN-Tester] Correctly specify React Native dependency (yarn berry) Jan 31, 2024
@Saadnajmi Saadnajmi changed the title [RN-Tester] Correctly specify React Native dependency (yarn berry) [RN-Tester] Correctly specify React Native dependency Jan 31, 2024
@Saadnajmi Saadnajmi changed the title [RN-Tester] Correctly specify React Native dependency [RNTester] Correctly specify React Native dependency Feb 2, 2024
@Saadnajmi Saadnajmi merged commit e24bfad into microsoft:main Feb 2, 2024
20 checks passed
@Saadnajmi Saadnajmi deleted the yarn-4-fix branch February 2, 2024 00:05
Saadnajmi added a commit to Saadnajmi/react-native-macos that referenced this pull request Feb 12, 2024
@Saadnajmi Saadnajmi mentioned this pull request Feb 12, 2024
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.

2 participants