This repository has been archived by the owner on Sep 11, 2018. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
refactor(server): koa -> express, deprecate use of babel on server
- Loading branch information
David Zukowski
committed
Sep 12, 2016
1 parent
15f703f
commit f00ebd3
Showing
17 changed files
with
119 additions
and
310 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,24 +1,29 @@ | ||
import fs from 'fs-extra' | ||
import _debug from 'debug' | ||
import webpackCompiler from '../build/webpack-compiler' | ||
import webpackConfig from '../build/webpack.config' | ||
import config from '../config' | ||
const fs = require('fs-extra') | ||
const debug = require('debug')('app:bin:compile') | ||
const webpackCompiler = require('../build/webpack-compiler') | ||
const webpackConfig = require('../build/webpack.config') | ||
const config = require('../config') | ||
|
||
const debug = _debug('app:bin:compile') | ||
const paths = config.utils_paths | ||
|
||
;(async function () { | ||
try { | ||
debug('Run compiler') | ||
const stats = await webpackCompiler(webpackConfig) | ||
if (stats.warnings.length && config.compiler_fail_on_warning) { | ||
debug('Config set to fail on warning, exiting with status code "1".') | ||
const compile = () => { | ||
debug('Starting compiler.') | ||
return Promise.resolve() | ||
.then(() => webpackCompiler(webpackConfig)) | ||
.then(stats => { | ||
if (stats.warnings.length && config.compiler_fail_on_warning) { | ||
throw new Error('Config set to fail on warning, exiting with status code "1".') | ||
} | ||
debug('Copying static assets to dist folder.') | ||
fs.copySync(paths.client('static'), paths.dist()) | ||
}) | ||
.then(() => { | ||
debug('Compilation completed successfully.') | ||
}) | ||
.catch((err) => { | ||
debug('Compiler encountered an error.', err) | ||
process.exit(1) | ||
} | ||
debug('Copy static assets to dist folder.') | ||
fs.copySync(paths.client('static'), paths.dist()) | ||
} catch (e) { | ||
debug('Compiler encountered an error.', e) | ||
process.exit(1) | ||
} | ||
})() | ||
}) | ||
} | ||
|
||
compile() |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,11 +1,9 @@ | ||
import config from '../config' | ||
import server from '../server/main' | ||
import _debug from 'debug' | ||
const config = require('../config') | ||
const server = require('../server/main') | ||
const debug = require('debug')('app:bin:server') | ||
|
||
const debug = _debug('app:bin:server') | ||
const port = config.server_port | ||
const host = config.server_host | ||
|
||
server.listen(port) | ||
debug(`Server is now running at http://${host}:${port}.`) | ||
debug(`Server accessible via localhost:${port} if you are using the project defaults.`) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
f00ebd3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davezuko Curious, did you guys run into issues with Koa2+Babel in production? All the recent simplification looks great btw.
f00ebd3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bhj no issues with Koa, and it's by far the saddest loss with the simplifications. The backend team at TA still uses Express but started ripping babel out and it's convinced me to do the same for personal projects, which means dropping Koa just because I'm not much of a fan of the generator async hackery despite its brilliance. Ultimately, the server in this project is just there for webpack in development and a rudimentary backend API. Anything bigger than that it's totally feasible to use whatever you want.
Babel did cause various headaches, if not outright issues, and at a minimum just made our lives more difficult (accurate code coverage reports, breaking changes/dependencies, inability to just start coding without any setup). It's a great idea, but I find that I'm distancing myself from complex build systems with each passing day.
And thanks, I appreciate it. I started more seriously questioning why all this stuff was in the project other than that it's part of the ecosystem and was requested at one point in time, so hopefully we can make things a little more manageable.
f00ebd3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davezuko Thanks and 100% support the philosophy. Maybe in a year or two we can finally use async/await properly!
f00ebd3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bhj Just a heads up that async await will be added in the next month or so @davezuko
nodejs/promises#4 (comment)
https://www.chromestatus.com/feature/5643236399906816
Might want to revert back to koa in December
f00ebd3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jonricaurte I've been using the Node 7 betas with
--harmony_async_await
for a few days and so far no problems! Will be interesting to see how long it stays behind a flag.f00ebd3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@bhj Hopefully not for long! :)
f00ebd3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the heads up @jonricaurte, would love to use Koa natively.
f00ebd3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Giving a little feedback since my experience has been different than what's expressed in this thread.
First, thank you for this project. It has helped me a TON in getting familiar with this stack; I wouldn't be where I am without this work. I recently made the jump over to Express by pulling in latest. I loved the gains I got in my learning because it's so much easier to find examples, tutorials etc for Express compared to Koa.
I'm bummed about losing the es6 stuff though. That was what I focused on learning first when I started modifying this project, and I've written quite a bit of code that leverages it. Especially bummed that the commit that does Koa->Express is the same as the one that nixes babel, as I now have to comb through this line by line to try to figure out how to get my babel functionality back (a piece of the stack which I've not yet devoted any time to). I'll solve this and move on with my project, but it was an unexpected hiccup and took a bit of the wind out of my sails after being so high on my progress gains from moving to Express.
No expectations here, just a little honest feedback for you guys. Again, I'm super thankful for this project and owe you a ton for how much I've learned. Thanks again for your hard work!
f00ebd3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for the feedback @joshuaandrewhoffman, it's always nice to hear others' perspectives. I understand it's a super controversial decision, but hopefully one that at least some will understand (or at least hate less) over time. The latest Node versions do support a lot of ES6 features, so I think you'll be surprised at what exactly you need Babel for. It is painful to lose those unsupported features, as some of them are striking (CommonJS vs ES6 modules being the most obvious) but you also gain a lot by having code that can be run natively without having to deal with the headaches caused by transpilation (portability, debugging, etc.).
Trust me, I wish as much as you that I could write the same exact type of code on the client as the server, but that's one of the flaws of our ecosystem. I tried for so long to use whatever tools made sense to lessen this gap, but the end result never felt right. With the backend at TechnologyAdvice completely dropping Babel from their stack, I've kind of had a heart-to-heart with the state of tooling. Babel is awesome, and great, and makes writing code for the browser not an absolutely soul killing experience, but if I can get away without using it I will.
Hopefully that sheds some pre-coffee probably-super-rambly light on the decision. Best of luck with everything.
f00ebd3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@davezuko thank you for taking the time to respond, your reply definitely helped me understand your decision. You're spot on that modules was the first thing I stumbled into. I've decided to give things a go the way you're advocating for and see how it works out; so far so good!
I really do want to stress again how appreciative I am of this project. I absolutely would not be where I am in my understanding of this stack without your effort, so thank you!
f00ebd3
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is a good move for this project, simplifying, but a word of warning to those (like me) that depend on the dependency
babel-polyfill
for non-es6 browsers like IE11, etc. I use axios and IE throwsUnhandled promise rejection ReferenceError: 'Promise' is undefined
after this change. So I had to add babel-polyfill back as a dependency and vendor file.