[WIP] Add minimal boilerplate #120

chakrihacker · 2018-04-09T19:04:51Z

No description provided.

DevanB · 2018-04-09T20:07:12Z

@chakrihacker Thanks for submitting this PR! We already have a PR open for a minimal boilerplate that is very similar to your PR, but had one main problem, the ApolloClient uri.

Here is the PR #72

And here is the blocking issue for which we haven't accepted many updates lately: #98

chakrihacker · 2018-04-10T04:26:36Z

Hi @DevanB thanks for pointing out existing PR. I am using expo constants to find dev url,so this makes sure even your emulator/simulator is in wifi/lan it connects to server.

DevanB · 2018-04-11T01:49:31Z

@chakrihacker - thanks for updating your PR. I’ll review within the next few days.

I might have a few more adjustments before accepting and merging. But if you’ll work through them, we will get your hard work merged in!

Thanks for submitting the PR!

chakrihacker · 2018-04-11T06:17:48Z

Sure No problem!!!

DevanB · 2018-04-16T03:55:11Z

@chakrihacker

I started looking over your PR this evening. Can you refactor your PR to align with the same folder structure we have for the basic example and the PR I have mentioned (this one)?

chakrihacker · 2018-04-19T17:52:31Z

Hey, I modified structure can you check??

chakrihacker · 2018-05-01T14:35:25Z

Hello any updates on this??

alecperkey · 2018-05-02T22:25:22Z

minimal/package.json

+{
+  "main": "node_modules/expo/AppEntry.js",
+  "private": true,
+  "dependencies": {


PR works for me on Windows 10 w/ exp cli commands exp lan --start followed by exp android

Perhaps the exp cli scripts can be added as npm scripts (for transparency to those unfamiliar with exp cli requirements)

something like:

"scripts": { "start": "react-native-scripts start", "exp:start:lan": "exp start --lan", "exp:android": "exp android", "exp:ios": "exp ios", "eject": "react-native-scripts eject", "android": "react-native-scripts android", "ios": "react-native-scripts ios", "test": "node node_modules/jest/bin/jest.js --watch" },

DevanB

Thanks for updating this PR, however, it is still missing vital pieces to align it with the basic boilerplate. For instance, the package.json is missing npm scripts.

Please take a look at the basic boilerplate and try to mimic it as much as possible. Thanks! 😄

alecperkey · 2018-05-07T18:11:15Z

Looks good to merge

DevanB · 2018-05-07T19:47:53Z

minimal/.gitignore

@@ -0,0 +1,3 @@
+node_modules/**/*
+.expo/*


We should change this to reflect the basic boilerplate. No need to globs to be used; it could just be node_modules/ and .expo/

DevanB · 2018-05-07T19:51:44Z

minimal/App.js

@@ -0,0 +1,23 @@
+import React from 'react';
+import { Text, View } from 'react-native';
+import Expo from "expo";


I'd love to see Constants imported specifically, instead of importing all of Expo.

DevanB · 2018-05-07T19:53:37Z

minimal/README.md

@@ -0,0 +1,104 @@
+


This file has an assortment of updates that are necessary for the minimal boilerplate. Please review. 😄

DevanB · 2018-05-07T19:56:51Z

minimal/app.json

@@ -0,0 +1,27 @@
+{


I'd love to see this file more complete, rather than the defaults that Expo provides.

I think the defaults are more than enough

DevanB · 2018-05-07T20:01:09Z

minimal/package.json

+  "private": true,
+  "scripts": {
+    "start": "react-native-scripts start",
+    "exp:start": "exp start --lan",


What does --lan afford users versus react-native-scripts start?

Since we are depending on local ip rather than domain url, it's necessary to use --lan to make that app has access to prisma server

DevanB · 2018-05-07T20:07:50Z

minimal/package.json

+    "graphql-tag": "^2.8.0",
+    "react": "16.3.0-alpha.1",
+    "react-apollo": "^2.1.3",
+    "react-native": "https://github.com/expo/react-native/archive/sdk-26.0.0.tar.gz"


Version 27 would change this to: "https://github.com/expo/react-native/archive/sdk-27.0.0.tar.gz"

DevanB · 2018-05-07T20:08:10Z

minimal/package.json

+  },
+  "dependencies": {
+    "apollo-boost": "^0.1.4",
+    "expo": "^26.0.0",


Version 27 would change this to: "^27.0.0"

DevanB · 2018-05-07T20:57:41Z

minimal/package.json

+    "expo": "^26.0.0",
+    "graphql": "^0.13.2",
+    "graphql-tag": "^2.8.0",
+    "react": "16.3.0-alpha.1",


Version 27 would change this to "16.3.1"

DevanB · 2018-05-07T20:58:28Z

minimal/package.json

+    "eslint-plugin-jsx-a11y": "6.0.3",
+    "eslint-plugin-react": "7.7.0",
+    "eslint-plugin-react-native": "3.2.1",
+    "jest-expo": "25.1.0",


Version 27 would change this to "^27.0.0"

DevanB · 2018-05-07T20:59:58Z

minimal/server/.gitignore

@@ -0,0 +1,2 @@
+node_modules
+package.json.lock


Would love to make this identical to the basic server's .gitignore.

DevanB

Okay, I'm back from vacation and gave this a super thorough review. Feel free to respond to my comments and start a conversation. I don't want to be a gatekeeper, but do want to have a healthy discussion, a healthy group of contributors, and to maintain code reuse and best practices between all React Native boilerplates.

DevanB · 2018-05-07T21:03:01Z

minimal/server/package.json

+    "prisma",
+    "graphcool"
+  ],
+  "author": "Subramanya Chakravarthy",


Please remove this line 😄

DevanB · 2018-05-07T21:17:35Z

minimal/server/package.json

@@ -0,0 +1,16 @@
+{


Please add the prettier settings from the basic server's package.json.

DevanB · 2018-05-07T21:21:56Z

minimal/src/Components/Home.js

+import gql from 'graphql-tag';
+import InputName from './InputName';
+
+export class Home extends Component {


Because this component only has a render method, please convert it to a functional component.

DevanB · 2018-05-07T21:22:10Z

minimal/src/Components/Home.js

+      <View style={styles.container} >
+        <Query query={HELLO_QUERY} >
+          {props => {
+            {/* console.log(props) */ }


Please remove this line 😄

DevanB · 2018-05-07T21:22:53Z

minimal/App.js

+  uri: `http://${url}`,
+})
+
+export default class App extends React.Component {


Because this component only has a render method, please convert it to a functional component.

marktani · 2018-05-07T22:18:24Z

Thanks a lot for your contribution @chakrihacker! 💪
Great that you identified all those improvements in your review, @DevanB! Let's get this polished and merged 😊

DevanB · 2018-05-07T22:28:03Z

Totally agree! Let's get this merged in and we can move on to the advanced boilerplate, which will be awesome to release to other developers!

chakrihacker · 2018-05-20T14:03:22Z

Hey @DevanB I have updated code based on your reviews. can we merge this.

chakrihacker · 2018-07-02T11:16:01Z

Hi, Any updates on this??

chakrihacker added 2 commits April 10, 2018 00:29

Add minimal boilerplate

ccaa83b

Add minimal boilerplate

675c62a

Using lan ip

bcb98d3

Refactor folder structure

fe2c38d

alecperkey reviewed May 2, 2018

View reviewed changes

DevanB suggested changes May 3, 2018

View reviewed changes

Fix: npm scripts

3f8aeb9

DevanB reviewed May 7, 2018

View reviewed changes

DevanB suggested changes May 7, 2018

View reviewed changes

Feat: Reviews

ae143e7

DevanB changed the title ~~Add minimal boilerplate~~ [WIP] Add minimal boilerplate May 8, 2018

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Add minimal boilerplate #120

[WIP] Add minimal boilerplate #120

chakrihacker commented Apr 9, 2018

DevanB commented Apr 9, 2018

chakrihacker commented Apr 10, 2018

DevanB commented Apr 11, 2018

chakrihacker commented Apr 11, 2018

DevanB commented Apr 16, 2018

chakrihacker commented Apr 19, 2018

chakrihacker commented May 1, 2018

alecperkey May 2, 2018

DevanB left a comment

alecperkey commented May 7, 2018

DevanB May 7, 2018

DevanB May 7, 2018

DevanB May 7, 2018

DevanB May 7, 2018

chakrihacker May 8, 2018

DevanB May 7, 2018

chakrihacker May 8, 2018

DevanB May 7, 2018

DevanB May 7, 2018

DevanB May 7, 2018

DevanB May 7, 2018

DevanB May 7, 2018

DevanB left a comment

DevanB May 7, 2018

DevanB May 7, 2018

DevanB May 7, 2018

DevanB May 7, 2018

DevanB May 7, 2018

marktani commented May 7, 2018

DevanB commented May 7, 2018

chakrihacker commented May 20, 2018

chakrihacker commented Jul 2, 2018

		@@ -0,0 +1,3 @@
		node_modules/*/
		.expo/*

		@@ -0,0 +1,2 @@
		node_modules
		package.json.lock

[WIP] Add minimal boilerplate #120

Are you sure you want to change the base?

[WIP] Add minimal boilerplate #120

Conversation

chakrihacker commented Apr 9, 2018

DevanB commented Apr 9, 2018

chakrihacker commented Apr 10, 2018

DevanB commented Apr 11, 2018

chakrihacker commented Apr 11, 2018

DevanB commented Apr 16, 2018

chakrihacker commented Apr 19, 2018

chakrihacker commented May 1, 2018

Choose a reason for hiding this comment

DevanB left a comment

Choose a reason for hiding this comment

alecperkey commented May 7, 2018

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DevanB 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

Choose a reason for hiding this comment

marktani commented May 7, 2018

DevanB commented May 7, 2018

chakrihacker commented May 20, 2018

chakrihacker commented Jul 2, 2018