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

Fixed webpack setup to seperate compilated client and server code #95

Merged
merged 4 commits into from Jul 1, 2017

Conversation

jkettmann
Copy link
Contributor

Changed build directories for client and server.

server -> build
client -> build/app

build/app is exposed via express.static, thus only client side code can be accessed publicly

Closes #94

@lvarayut
Copy link
Owner

lvarayut commented Jun 30, 2017

Thanks @jkettmann for the awesome PR. Could you change the branch to canary instead of master, you should be able to do so by clicking the lvarayut:master at the top and change it to lvarayut:canary?

@jkettmann
Copy link
Contributor Author

@lvarayut Yes, of course. Can you check your heroku deploy? Not sure if I broke that.

@jkettmann jkettmann changed the base branch from master to canary June 30, 2017 14:57
Copy link
Owner

@lvarayut lvarayut left a comment

Choose a reason for hiding this comment

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

👍 , just left some curious comments 😃

package.json Outdated
@@ -97,7 +97,7 @@
"react-router-relay": "^0.14.0",
"style-loader": "^0.18.2",
"url-loader": "^0.5.9",
"webpack": "^3.0.0",
"webpack": "2",
Copy link
Owner

Choose a reason for hiding this comment

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

Could you explain why do you downgrade the version of webpack?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually I couldn't start with webpack 3. Doesn't have anything to do with this issue though.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@jkettmann Also having trouble starting new webpack 3 something along these lines?

ERROR in   TypeError: Cannot read property 'request' of undefined
  
  - ExternalModuleFactoryPlugin.js:37 handleExternals
    [relay-fullstack]/[html-webpack-plugin]/[webpack]/lib/ExternalModuleFactoryP    lugin.js:37:33
  
....

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Owner

Choose a reason for hiding this comment

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

@ncrmro and @jkettmann, those are very useful information

package.json Outdated
@@ -9,7 +9,7 @@
"build-server": "cross-env NODE_ENV=production ./node_modules/.bin/babel ./server --out-dir ./build",
"lint": "eslint client server",
"flow": "flow",
"heroku-postbuild": "cross-env NODE_ENV=production webpack --config webpack.config.js && cross-env NODE_ENV=production ./node_modules/.bin/babel ./server --out-dir ./lib",
"heroku-postbuild": "cross-env NODE_ENV=production webpack --config webpack.config.js && cross-env NODE_ENV=production ./node_modules/.bin/babel ./server --out-dir ./build",
Copy link
Owner

@lvarayut lvarayut Jun 30, 2017

Choose a reason for hiding this comment

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

Do we need to change the output path of the Heroku app?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. This is not necessary.

@@ -41,6 +43,7 @@ if (process.env.NODE_ENV === 'production') {
'webpack-dev-server/client?http://localhost:3000',
'webpack/hot/only-dev-server'
];
outputPath = path.join(__dirname, 'app');
Copy link
Owner

Choose a reason for hiding this comment

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

Do we need to use the different path for the development environment?

Copy link
Contributor Author

@jkettmann jkettmann Jun 30, 2017

Choose a reason for hiding this comment

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

You're right again. Is the contentBase and static server in server/index.js necessary (if I remove them everything seems to be still working). I guess that's why I used a different path for development.

const relayServer = new WebpackDevServer(webpack(webpackConfig), {
    contentBase: '/build/',

...

relayServer.use('/', express.static(path.join(__dirname, '../build/app')));

@jkettmann
Copy link
Contributor Author

@lvarayut I removed the unnecessesary changes.

@lvarayut
Copy link
Owner

lvarayut commented Jul 1, 2017

Thanks @jkettmann 👍

@lvarayut lvarayut merged commit 073c1be into lvarayut:canary Jul 1, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants