Add docker to yeoman generator #200

withinboredom · 2016-05-21T21:36:26Z

This PR adds a simple docker-compose.yml file and a simple Dockerfile to the yeoman generator to allow people to quickly get up and running with Docker. This will be helpful for developers using Windows.

CLAassistant · 2016-05-21T21:36:27Z

All committers have signed the CLA.

gigabo · 2016-05-21T22:11:38Z

Yeah! 🤘

This is awesome! I need to get set up with a modern docker to try it out.

@doug-wade - You're most familiar with this yeoman generator. Take a look at this!

gigabo · 2016-05-21T22:14:11Z

packages/generator-react-server/generators/app/templates/README.md

 npm start
 ```

 Then go to [localhost:3000](http://localhost:3000/). You will see a simple page that pre-renders and that is interactive on load. It also will include hot reloading of React components in their own file.

 If you want to optimize the client code at the expense of startup time, type `NODE_ENV=production npm start`. You can also use any of [the other arguments for react-server-cli](../../react-server-cli#setting-options-manually) after `--`. For example:

-```
+```shell


Thanks. Nice to have syntax coloring on these.

👍 Thanks for adding this!

gigabo · 2016-05-22T15:55:20Z

packages/generator-react-server/generators/app/templates/README.md

+  "env": {
+    "docker": {
+      "host": "Your ip from `docker-machine ip` here"
+    },


This is cool. Nice use of environment configuration.

Thanks for the added documentation. Clears it right up for me.

gigabo · 2016-05-22T16:01:24Z

This looks great to me. 👍

I'd like to wait until @doug-wade has had a chance to look over it, too, since he knows this generator well.

@doug-wade - Is this something that would be easy for us to prompt the user for down the road? Like want to run in a docker container? Yes/no and conditionally include this stuff?

gigabo · 2016-05-23T20:20:12Z

packages/generator-react-server/generators/app/templates/README.md

+      "port": "80"
+    }
+  }
+}


Just occurred to me that we should probably generate this config. #203

gigabo · 2016-05-23T20:37:18Z

Hey, I just tried this locally. The README changes showed up, but the dockerfile and the docker-compose.yml didn't... @doug-wade is there something else that needs to happen for new files to get picked up?

doug-wade · 2016-05-23T21:12:19Z

@withinboredom This is pretty awesome! As @gigabo notes, there are still some things we'll want you to do before you merge

I don't match files with globs currently, even though I should. For now, you'll need to enumerate the new files in one of the two lists of files that are operated on by the generator (one is for dotfiles that are used by git and babel for configuration, which I preface with _ rather than . to prevent them interacting with the monorepo; you may want to add your docker config there to make it easier to develop on Docker, though I'm not sure; if there isn't a risk of interaction, there is also one for files that are not renamed.
Not all clients will want to include a Docker container with their new react-server project, so we should prompt them (Yeoman uses Inquirer for prompting ), and only generate the Docker config if the user asks for it (my first intuition is to default to no Docker config, personally, since we push the Yeoman generator as an easy way to get started, and the Docker config makes react-server look bigger and scarier to a new developer; honestly I should put the linter behind an Inquirer prompt for the same reason)

withinboredom · 2016-05-24T00:36:08Z

@doug-wade,

I added the docker files as a prompt, and included changes to the unit tests. I can't get the new test (where dockerCfg is true) to pass, although I tested locally and it seems to function as expected. Maybe you can take a gander and give me a hint as to what I did wrong?

withinboredom · 2016-05-24T00:37:21Z

Also, it seems like a good idea to wait for #203 before generating the config for a dockerish runtime and I think its a good idea to leave that in the README for now. What do you think?

gigabo · 2016-05-24T00:40:10Z

wait for #203 before generating the config

Makes sense.

gigabo · 2016-05-24T01:12:36Z

Alright! It's loading for me!

But... static assets aren't, yet.

Still trying to fetch through localhost. Did I mess up my config, or is there a bug somewhere?

$ cat .reactserverrc
{
  "port": "3000",
  "env": {
    "docker": {
      "host": "192.168.99.100"
    }
  }
}

withinboredom · 2016-05-24T02:18:24Z

I had issues with the config as well, I figured it was environmental ... guess I should open an issue. You can modify the script in package.json, here's mine for example:

{
  "name": "reactFrontend",
  "version": "0.0.1",
  "description": "A react-server instance",
  "main": "HelloWorld.js",
  "scripts": {
    "start": "react-server-cli --host 192.168.99.100", 
    "test": "xo && nsp check"
  },
  "license": "Apache-2.0",
  "dependencies": {
    "babel-plugin-transform-runtime": "^6.8.0",
    "babel-preset-es2015": "^6.6.0",
    "babel-preset-react": "^6.5.0",
    "babel-runtime": "^6.6.1",
    "react": "~0.14.2",
    "react-dom": "~0.14.2",
    "react-server": "^0.2.10",
    "react-server-cli": "^0.2.10",
    "superagent": "1.2.0"
  },
  "devDependencies": {
    "eslint-config-xo-react": "^0.7.0",
    "eslint-plugin-react": "^5.1.1",
    "nsp": "^2.3.3",
    "xo": "^0.15.1"
  },
  "xo": {
    "esnext": true,
    "extends": "xo-react",
    "ignores": ["__clientTemp/**/*"]
  }
}

withinboredom · 2016-05-24T02:26:20Z

You can verify that the environment is set right by changing the hello world:

Hello, World: {process.env.NODE_ENV}

withinboredom · 2016-05-24T02:32:31Z

I think I found the bug ... trying a fix and PR if that was it. Not really setup to debug CLI scripts, so console.logging all over ;)

withinboredom · 2016-05-24T04:16:32Z

Yep, fixed in #206 ... that was a fun one.

gigabo · 2016-05-24T17:16:13Z

modify the script in package.json

Sweet, that worked. Thanks.

Mind rebasing this branch to pick up your fix from #206 now that it's merged?

I'll see if I can track down what's going wrong with the tests.

gigabo · 2016-05-24T17:43:29Z

@doug-wade - Uhh.... why aren't the tests passing here? Seems right.

doug-wade · 2016-05-24T19:53:16Z

@withinboredom The bug was in the tests, not in your changes 🐑 . #207 should unblock you

doug-wade · 2016-05-24T23:37:39Z

@withinboredom I think its failing with a (well-obscured) linter failure; I've added a line comment

doug-wade · 2016-05-24T23:38:20Z

packages/generator-react-server/generators/app/index.js

@@ -14,6 +14,11 @@ module.exports = yeoman.Base.extend({
 			name: 'name',
 			message: 'What would you like to call your app?',
 			default: this.appname
+		},{


I think this is causing the linter failure that's failing your build; it should be }, {

@doug-wade - How did you track that down? I don't see anything about it in travis...

I ran the branch locally to find #207 and when I verified that it made the existing tests pass, I noticed there was still a linter failure.

doug-wade · 2016-05-25T01:02:35Z

lgtm 👍
@gigabo anything else before we merge this?

gigabo · 2016-05-25T01:06:52Z

This is awesome! Thanks @withinboredom!

I hope we'll see more cool stuff from you soon. 👹

gigabo · 2016-05-25T01:09:32Z

BTW - I'm testing out a changelog generator.

The next version's shaping up to be the @withinboredom show:

Unreleased (2016-05-25)

Bug fix

react-server-cli
- #206 Default options for cli overwrite config file options. (@withinboredom)

Enhancement

generator-react-server
- #200 Add docker to yeoman generator. (@withinboredom)

Commiters: 1

withinboredom

gigabo reviewed May 21, 2016
View reviewed changes

gigabo added the enhancement New functionality. label May 21, 2016

gigabo reviewed May 22, 2016
View reviewed changes

gigabo reviewed May 23, 2016
View reviewed changes

withinboredom mentioned this pull request May 24, 2016

Default options for cli overwrite config file options #206

Merged

doug-wade mentioned this pull request May 24, 2016

Better Generator Tests #207

Merged

withinboredom added 4 commits May 24, 2016 19:20

Add docker to yeoman generator

5f7f60f

Updated README and comments

1f97b34

Got to love GitHub syntax highlighting

1df765d

Added dockerfile as a prompt and added unit tests

12be333

withinboredom force-pushed the master branch from e9399f0 to 12be333 Compare May 24, 2016 23:22

doug-wade reviewed May 24, 2016
View reviewed changes

Fix style issue

b4fa999

gigabo merged commit e5f96fb into redfin:master May 25, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add docker to yeoman generator #200

Add docker to yeoman generator #200

withinboredom commented May 21, 2016 •

edited

Loading

CLAassistant commented May 21, 2016 •

edited

Loading

gigabo commented May 21, 2016

gigabo May 21, 2016

doug-wade May 23, 2016

gigabo May 22, 2016 •

edited

Loading

gigabo commented May 22, 2016

gigabo May 23, 2016

gigabo commented May 23, 2016

doug-wade commented May 23, 2016

withinboredom commented May 24, 2016

withinboredom commented May 24, 2016 •

edited

Loading

gigabo commented May 24, 2016

gigabo commented May 24, 2016

withinboredom commented May 24, 2016

withinboredom commented May 24, 2016

withinboredom commented May 24, 2016

withinboredom commented May 24, 2016

gigabo commented May 24, 2016

gigabo commented May 24, 2016

doug-wade commented May 24, 2016

doug-wade commented May 24, 2016

doug-wade May 24, 2016

gigabo May 24, 2016

doug-wade May 24, 2016

doug-wade commented May 25, 2016 •

edited

Loading

gigabo commented May 25, 2016

gigabo commented May 25, 2016

Add docker to yeoman generator #200

Add docker to yeoman generator #200

Conversation

withinboredom commented May 21, 2016 • edited Loading

CLAassistant commented May 21, 2016 • edited Loading

gigabo commented May 21, 2016

gigabo May 21, 2016

Choose a reason for hiding this comment

doug-wade May 23, 2016

Choose a reason for hiding this comment

gigabo May 22, 2016 • edited Loading

Choose a reason for hiding this comment

gigabo commented May 22, 2016

gigabo May 23, 2016

Choose a reason for hiding this comment

gigabo commented May 23, 2016

doug-wade commented May 23, 2016

withinboredom commented May 24, 2016

withinboredom commented May 24, 2016 • edited Loading

gigabo commented May 24, 2016

gigabo commented May 24, 2016

withinboredom commented May 24, 2016

withinboredom commented May 24, 2016

withinboredom commented May 24, 2016

withinboredom commented May 24, 2016

gigabo commented May 24, 2016

gigabo commented May 24, 2016

doug-wade commented May 24, 2016

doug-wade commented May 24, 2016

doug-wade May 24, 2016

Choose a reason for hiding this comment

gigabo May 24, 2016

Choose a reason for hiding this comment

doug-wade May 24, 2016

Choose a reason for hiding this comment

doug-wade commented May 25, 2016 • edited Loading

gigabo commented May 25, 2016

gigabo commented May 25, 2016

Unreleased (2016-05-25)

Bug fix

Enhancement

Commiters: 1

withinboredom commented May 21, 2016 •

edited

Loading

CLAassistant commented May 21, 2016 •

edited

Loading

gigabo May 22, 2016 •

edited

Loading

withinboredom commented May 24, 2016 •

edited

Loading

doug-wade commented May 25, 2016 •

edited

Loading