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

2785 Updated the dependencies to the latest version #48

Closed
wants to merge 11 commits into from

Conversation

bkarambe
Copy link
Contributor

@bkarambe bkarambe commented Jan 7, 2022

Fixes #7

Changes proposed in this Pull Request:

  • Updated dependencies to the latest versions

To Test:

  • Create new cli projects

@jeffvg
Copy link
Contributor

jeffvg commented Jan 7, 2022

I normally run template files view npx with file location to stand these up but should the "start:blank-ios": "cd demo && yarn start:blank-ios" from package.json scripts be working? Received this error:

[!] CocoaPods could not find compatible versions for pod "FBReactNativeSpec":
  In Podfile:
    FBReactNativeSpec (from ../node_modules/react-native/React/FBReactNativeSpec)
 
    React-RCTImage (from ../node_modules/react-native/Libraries/Image) was resolved to 0.66.4, which depends on
      FBReactNativeSpec (= 0.66.4)
 
Specs satisfying the FBReactNativeSpec (from ../node_modules/react-native/React/FBReactNativeSpec), FBReactNativeSpec (= 0.66.4) dependency were found, but they required a higher minimum deployment target.

@daileytj
Copy link
Contributor

daileytj commented Jan 7, 2022

@jeffvg the templates should run a local demo with those scripts. Is it working in dev?

@jeffvg
Copy link
Contributor

jeffvg commented Jan 7, 2022

@jeffvg the templates should run a local demo with those scripts. Is it working in dev?

looking at dev right now but in feature branch running the local script spins up iPhone 13 on iOS 15.2. Using npx it spins up iPhone 12 iOS 14.2

the dev branch runs with local demo script no issues but it spins up iPhone 11 iOS 14.2 for me

@@ -44,7 +44,7 @@ export const App = (): JSX.Element => {
const { getInitialState } = useLinking(ref, authLinkMapping);
const [initialState, setInitialState] = React.useState();
React.useEffect(() => {
resolveInitialState(getInitialState, setInitialState);
resolveInitialState(getInitialState as () => any, setInitialState as (state: any) => void);
Copy link
Contributor

Choose a reason for hiding this comment

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

We shouldn't be using the any type

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a little tricky; getInitialState is the correct type already (even without the any cast) but the auth-ts template fails linting because it doesn't have access to the type definitions found within @react-navigation/native.

I'm confused as to why we're creating templates without having the required deps installed at the root level.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this story is going to evolve into something larger than expected; how is linting working without the type dependencies installed?

Copy link
Contributor

Choose a reason for hiding this comment

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

@daileytj do you have thoughts on this one? We might need to update the root package.json so it contains all the dependencies that each template requires; this is how the angular one does it currently.

Copy link
Contributor

Choose a reason for hiding this comment

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

The root package.json absolutely should contain every dependency that is in any template

Copy link
Contributor

Choose a reason for hiding this comment

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

This is how it was initially written so that the demo project could run every template locally for testing purposes

@daileytj
Copy link
Contributor

daileytj commented Jan 7, 2022

@jeffvg the templates should run a local demo with those scripts. Is it working in dev?

looking at dev right now but in feature branch running the local script spins up iPhone 13 on iOS 15.2. Using npx it spins up iPhone 12 iOS 14.2

I don't think we're controlling which emulator it opens in. I don't think it's super relevant to this story anyway.

@daileytj
Copy link
Contributor

daileytj commented Jan 7, 2022

This story is supposed to update the templates to the latest. All you did here was update the demo project dependencies. Be sure to update the template dependencies and make sure that each one builds with the cli locally as well as in the demo projects. I also believe we locked RN templates down to not be greater than 0.64 due to issues it introduces. If you can get everything building on 66 that would be great though.

@emclaug2
Copy link
Contributor

emclaug2 commented Jan 7, 2022

Be sure to update the template dependencies and make sure that each one builds with the cli locally as well as in the demo projects.

@bkarambe See /blank/dependencies.json for example.

@@ -27,18 +27,18 @@
"precommit": "cd demo && bash ./scripts/buildBlank.sh && cd .. && yarn lint && yarn prettier"
},
"dependencies": {
Copy link
Contributor

Choose a reason for hiding this comment

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

@bkarambe This package will need to be updated to contain latest versions of all deps for each template.

@daileytj
Copy link
Contributor

daileytj commented Jan 10, 2022

All the demos work, but when I run this with the cli I'm still having issues. I think we need to make some other decisions here first. We were running into issues on RN 0.65.* previously and decided to lock it down at 0.64.1 and wait to migrate to the latest versions across the board. There are some breaking changes in the latest versions of react-native and react-navigation(could be more, but these ones for sure) that will likely need to be updated in the auth-workflow first. So... we can finish this story by using the latest available packages that are compatible with 0.64 for now and revisit this later or we can pause this in its tracks and update auth-shared -> auth-workflow -> then this -> cli.

@joebochill @emclaug2 would like your thoughts.

edit: I didn't check to see what all needs updated specifically, but it would be more logical to start top-down on dependencies

@daileytj
Copy link
Contributor

Closing this PR. We will circle back around to this after updating to MUI v5 and updating the dependencies and files for auth-shared and auth-workflow packages.

@daileytj daileytj closed this Jan 11, 2022
@joebochill
Copy link
Collaborator

I agree with closing this for now. The linked issue is for the next @major version, and we have a number of items to release in a @minor prior to that. Additionally, a number of these major package updates will require additional configuration (e.g. reanimated v2) or extensive compatibility testing.

@major
Copy link

major commented Jan 11, 2022

I agree with closing this for now. The linked issue is for the next @major version, and we have a number of items to release in a @minor prior to that.

All of you are doing excellent work, but there's no need to name the next version after me. Just use a version number instead. 😜

@joebochill
Copy link
Collaborator

🤣

@major
Copy link

major commented Jan 11, 2022

https://media.giphy.com/media/Jpv9Siby42bD9gDxH9/giphy.gif

@jeffvg jeffvg deleted the bug/2785-change-deps-in-package branch June 15, 2022 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Update dependencies and peer dependencies
6 participants