Skip to content

fixing Heroku deployment #45

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,4 +105,27 @@ $ npm run clean

Removes the compiled app file from build.

## Deploying to Heroku

Build/deploy pipeline of this project relies on two environment variables:
```
NPM_CONFIG_PRODUCTION=false
PRODUCTION=true
```
When it comes to deploying to Heroku you have two options for deployment:

### With "Deploy to Heroku" button
If you deploy on Heroku with the button on top of this page, you don't need to change anything as environment varialbes are read from `app.json`

### With console
If you deploy to Heroku via command line (like in [this](https://devcenter.heroku.com/articles/getting-started-with-nodejs#deploy-the-app) tutorial), you will need to setup these 2 env variables.
In order to do this, execute next lines from console once you setup Heroku:

```
heroku config:set NPM_CONFIG_PRODUCTION=false
heroku config:set PRODUCTION=true
```

If you don't do this, Heroku will fail to build your app (during `git push heroku master`) as environment variables will not be set.

## [Changelog](CHANGELOG.md)
2 changes: 1 addition & 1 deletion server.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import express from 'express';
var express = require('express');
Copy link
Owner

Choose a reason for hiding this comment

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

This shouldn't be necessary since the top-level start.js file registers the Babel hook before requireing server.

Copy link
Author

@sizov sizov May 15, 2016

Choose a reason for hiding this comment

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

@pheuter thank you for reply, I understand the idea, but unfortunately without this change I was not able to run this on Heroku (and looks like I'm not the only one).

Please see your original app deployed with Heroku button here: https://original-essential-react.herokuapp.com/ (gives error)
And here is the same deployment to Heroku with button from my fork (1 line changed): https://fix-import-essential-react.herokuapp.com/ (works as expected)

Copy link
Owner

Choose a reason for hiding this comment

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

Hm, I feel hesitant merging this change in just because Heroku isn't properly configured to run the start.js script before running server.jsx, I think figuring that out would be a better change since it wouldn't require modifying application code.

const app = express();


Expand Down