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 Resolutions & Move to Yarn #185

Merged
merged 1 commit into from
Dec 18, 2023
Merged

Conversation

alexrisch
Copy link
Contributor

@alexrisch alexrisch commented Dec 16, 2023

Problem:
NPM forces us to jump through a number of hoops
We seen an issue when dealing with React peer dependencies but they do not get handled appropriately

This is actually also seen in new expo modules created through the create-expo-module cli

Solution:
Stop forcing dependencies and working around dependency resolution and simply use yarn

Changes:
Removed package-lock.json
updated docs to point user towards yarn
Fix readme for npm pod-install to npx pod-install

Fixes Issues with Hooks addition from this #178 & identified on here #177

Problem:
NPM forces us to jump through number of hoops
We seen an issue when dealing with React peer dependencies but they do not get handled appropriately

This is actually also seen in new expo modules created through the create-expo-module cli

Solution:
Stop forcing dependencies and working around dependency resolution and simply use yarn

Changes:
Removed package-lock.json
updated docs to point user towards yarn
Fix readme for npm pod-install to npx pod-install
@alexrisch alexrisch force-pushed the user/alexrisch-yarn-and-resolutions branch from a4e8fbb to f09b5fb Compare December 16, 2023 19:55
@rygine
Copy link
Contributor

rygine commented Dec 16, 2023

looks like this is using Yarn v1. why not Yarn v4?

Copy link
Contributor

@nplasterer nplasterer left a comment

Choose a reason for hiding this comment

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

Works perfectly! Yay for yarn. 🎉

@cameronvoell cameronvoell self-requested a review December 18, 2023 18:00
Copy link
Contributor

@cameronvoell cameronvoell left a comment

Choose a reason for hiding this comment

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

Agree that defaulting to yarn in this repo will simplify our expo modules + example app setup. PR looks ready to merge to me!

@alexrisch
Copy link
Contributor Author

looks like this is using Yarn v1. why not Yarn v4?

I still only have v1 setup and it still seems to be the stable version: https://classic.yarnpkg.com/lang/en/docs/install/#mac-stable
I'll do some reading, v1 should allow us with the easiest migration from npm

@rygine
Copy link
Contributor

rygine commented Dec 18, 2023

looks like this is using Yarn v1. why not Yarn v4?

I still only have v1 setup and it still seems to be the stable version: https://classic.yarnpkg.com/lang/en/docs/install/#mac-stable I'll do some reading, v1 should allow us with the easiest migration from npm

v1 has been in maintenance mode since 1/2020. latest version now is 4.0.2, which requires corepack to be enabled.

just need to run corepack enable, then yarn set version stable. be sure to update the nodeLinker setting to node-modules to disable PnP mode.

that should hopefully just work. there are some GH actions updates as well. happy to sync on this if you'd like.

@cameronvoell
Copy link
Contributor

I propose we merge this with yarn v1 because it's the yarn setup linked to from Expo docs, and we know it resolves a build issue that is currently merged on main, and the CI is working as is in this PR, and then we do a follow up issue for migrating from yarn v1 to yarn v4 => https://yarnpkg.com/migration/guide

That way we can resolve the build issue that is merged to main right now, without having to make sure that our expo setup and CLI has any issues with yarn v4. thoughts? @rygine @alexrisch

@rygine
Copy link
Contributor

rygine commented Dec 18, 2023

I propose we merge this with yarn v1

sounds good!

@alexrisch alexrisch merged commit e6053cd into main Dec 18, 2023
4 of 5 checks passed
@alexrisch alexrisch deleted the user/alexrisch-yarn-and-resolutions branch December 18, 2023 21:25
Copy link
Contributor

🎉 This PR is included in version 1.23.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants