Add React Navigation support #22

rakeshpetit · 2023-11-24T15:12:53Z

This PR is an attempt to add React Navigation and a stack navigator to the scaffolded Expo app.

Todo

Test yarn thoughtbelt navigation which I couldn't do.

Success flow

rakeshpetit · 2023-11-24T15:25:07Z

src/commands/navigation.ts

+      'npx expo install react-native-screens react-native-safe-area-context',
+    );
+  }
+  // what if it is not expo?


What do we do for non Expo projects?

Do we already have support for react-native-cli? Or just Expo? If the project is not expo we just go through the bare workflow with install react-native-screens react-native-safe-area-context but we might need to be sensitive to what is the package manager being used, like we do in this PR.

Our philosophy so far has been to try to support Expo and bare RN, but I'd say we are only doing active testing against Expo at this point. But yes, I agree with Diego that if the project is not Expo, we should just do what the React Navigation docs recommend. If we use our addDependency util, it will handle picking the right package manager.

rakeshpetit · 2023-12-11T11:39:16Z

@stevehanson - I updated the tests and everything passes fine on the generated app but thoughtbelt still seems to complain on the CI. Hope to pair and resolve this.

rakeshpetit · 2023-12-11T13:27:38Z

@stevehanson - I updated the tests and everything passes fine on the generated app but thoughtbelt still seems to complain on the CI. Hope to pair and resolve this.

I think I've fixed that in the latest update.

rakeshpetit · 2023-12-11T13:34:45Z

Things work well on my local machine but fails on the CI 😞

codeofdiego · 2023-12-11T13:49:45Z

@rakeshpetit Can you try adding the file name "./**/*.tsx" to "include": ["./**/*.ts", "./**/*.tsx"], in tsconfig.json?

codeofdiego · 2023-12-11T14:49:22Z

templates/reactNavigation/__tests__/App.test.tsx

+import { Home } from '../App';
+
+test('renders', () => {
+  expect(true).toBe(true);


Was this expectation required to make something work?

codeofdiego · 2023-12-11T14:53:31Z

templates/reactNavigation/App.tsx

+export function Home() {
+  return (
+    <View style={styles.container}>
+      <Text>Open up App.js to start working on your app!</Text>


Suggested change

<Text>Open up App.js to start working on your app!</Text>

<Text>Open up App.tsx to start working on your app!</Text>

Just a typo in the App file extension :)

rakeshpetit · 2023-12-11T14:53:39Z

@rakeshpetit Can you try adding the file name "./**/*.tsx" to "include": ["./**/*.ts", "./**/*.tsx"], in tsconfig.json?

Adding __tests__ to tsconfig solved the problem in commit

stevehanson

This is looking great! I left a few comments and would be happy to discuss or pair. A couple other thoughts that come to mind:

There are a few ways to type React Navigation projects. Do we want to set that up? (I vote yes! but could be a separate PR). I'm a strong believer that rigid types are essential for navigation, since some tests tend to mock out navigation calls and therefore never test that it works! I have a typing approach that worked fairly well for a recent project that I'd be happy to share
I'm curious what our thoughts are on eventually taking this a step further and implementing bottom tabs (probably after asking the user if they want them?) or a login screen. This isn't for this PR but would be an interesting discussion. It seems this functionality would help projects get off the ground quickly

stevehanson · 2023-12-11T18:04:40Z

templates/reactNavigation/App.tsx

+  );
+}
+
+export default function App() {


When we create a new app using thoughtbelt, we will want to generate an App.tsx that has all of our providers and tools configured, including React Navigation, global state provider, styling provider, etc. To me, since this file will be tied to many different commands, it would make more sense for the creation of this file to be a step at the end of the createApp command, independent of any one command.

If we run thoughtbelt navigation independently, we will not want to create a new App.tsx in that case, though it would likely be helpful to include some output directing the user to wrap their app in a NavigationContainer. What do you think?

We don't yet have a mechanism for detecting if a given command is running independently as opposed to being a part of the main createApp (or other) command .

I opened a draft PR here #25 .

stevehanson · 2023-12-11T18:05:57Z

templates/reactNavigation/App.tsx

+  );
+}
+
+function RootStackNavigator() {


I usually create a src/navigators directory for these. Curious what you prefer here? Ideally, thoughtbelt will be opinionated and create a file structure that makes sense as the app grows.

Great comment. I wanted us to have this discussion in this PR and intentionally put all the navigators in App.tsx. Let's pair on this.

- Refactor navigators to be in a navigators folder - Add navigator types and global types - Add HomeScreen

stevehanson · 2023-12-12T19:13:46Z

Taking a look at this now to see what's left to get it to the finish line.

Co-authored-by: Frida Casas <[email protected]>

fridaland

🎉

stevehanson

Great work!

rakeshpetit commented Nov 24, 2023

View reviewed changes

rakeshpetit force-pushed the add-react-navigation-command branch from e732ac6 to ff8d262 Compare December 11, 2023 10:57

rakeshpetit force-pushed the add-react-navigation-command branch from ff8d262 to d187d88 Compare December 11, 2023 13:26

codeofdiego reviewed Dec 11, 2023

View reviewed changes

rakeshpetit force-pushed the add-react-navigation-command branch from 81f63ed to 6b95dc5 Compare December 11, 2023 15:22

rakeshpetit changed the title ~~WIP - Add React Navigation support~~ Add React Navigation support Dec 11, 2023

rakeshpetit marked this pull request as ready for review December 11, 2023 15:59

rakeshpetit requested a review from stevehanson as a code owner December 11, 2023 15:59

stevehanson reviewed Dec 11, 2023

View reviewed changes

rakeshpetit added 5 commits December 12, 2023 17:07

Add support for react navigation with tests

a891e23

Rectify the test file path

89fcef9

Add __tests__ to tsconfig

9851d7a

Add navigation command to the list

4fa3012

Fix naming in the Text

e1b83d9

rakeshpetit force-pushed the add-react-navigation-command branch from 93ccb2a to e1b83d9 Compare December 12, 2023 17:07

Changes for navigation

91b4046

- Refactor navigators to be in a navigators folder - Add navigator types and global types - Add HomeScreen

Stephen Hanson and others added 5 commits December 12, 2023 13:36

Move App test

6a809b8

Fix test

b40fec5

Fix test

6d2af96

Commit templates

94a4248

Co-authored-by: Frida Casas <[email protected]>

Remove unneeded tsconfig change

f0fd02e

fridaland approved these changes Dec 12, 2023

View reviewed changes

stevehanson merged commit f536334 into main Dec 12, 2023
2 checks passed

stevehanson deleted the add-react-navigation-command branch December 12, 2023 21:57

stevehanson reviewed Dec 12, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add React Navigation support #22

Add React Navigation support #22

rakeshpetit commented Nov 24, 2023 •

edited

Loading

rakeshpetit Nov 24, 2023

codeofdiego Dec 11, 2023 •

edited

Loading

stevehanson Dec 11, 2023

rakeshpetit commented Dec 11, 2023

rakeshpetit commented Dec 11, 2023

rakeshpetit commented Dec 11, 2023

codeofdiego commented Dec 11, 2023

codeofdiego Dec 11, 2023

codeofdiego Dec 11, 2023

rakeshpetit commented Dec 11, 2023

stevehanson left a comment

stevehanson Dec 11, 2023

stevehanson Dec 11, 2023

stevehanson Dec 11, 2023

rakeshpetit Dec 12, 2023

stevehanson commented Dec 12, 2023

fridaland left a comment

stevehanson left a comment

	<Text>Open up App.js to start working on your app!</Text>
	<Text>Open up App.tsx to start working on your app!</Text>

Add React Navigation support #22

Add React Navigation support #22

Conversation

rakeshpetit commented Nov 24, 2023 • edited Loading

Todo

Success flow

Choose a reason for hiding this comment

codeofdiego Dec 11, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rakeshpetit commented Dec 11, 2023

rakeshpetit commented Dec 11, 2023

rakeshpetit commented Dec 11, 2023

codeofdiego commented Dec 11, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rakeshpetit commented Dec 11, 2023

stevehanson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

stevehanson commented Dec 12, 2023

fridaland left a comment

Choose a reason for hiding this comment

stevehanson left a comment

Choose a reason for hiding this comment

rakeshpetit commented Nov 24, 2023 •

edited

Loading

codeofdiego Dec 11, 2023 •

edited

Loading