-
Notifications
You must be signed in to change notification settings - Fork 122
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
[GPT-432] Package server webpack #346
Changes from all commits
b56da0a
f39644b
d7be7b5
7faf403
8ffe4cf
de55e78
abb5f83
e268992
18e6ce7
99b4b84
5dc0060
f5d80b3
8a71702
a1db055
c0d9c6b
d80bfa2
c24d039
4a17d9a
d0d1574
46af301
2aeeb80
3ca88c7
7e1cbdc
af9b87f
89aafd3
1bb52c7
fd0c275
17ef3b3
afdb51e
c312a54
d97575c
494a8b1
0b491bb
4939afd
818f58a
8944a47
9b3fefe
366200e
19477b3
2a9cd22
41944ce
dbf4714
34dc1ce
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,36 @@ | ||
/* | ||
* External Dependencies handler for webpack | ||
* Returns an array with handlers to indicates dependencies that should not be | ||
* bundled by webPack but instead remain requested by the resulting bundle. | ||
* For more info http://webpack.github.io/docs/configuration.html#externals | ||
* | ||
*/ | ||
'use strict'; | ||
var format = require('stringformat'); | ||
var _ = require('underscore'); | ||
var strings = require('../../../../../resources'); | ||
|
||
|
||
module.exports = function externalDependenciesHandlers(dependencies){ | ||
var deps = dependencies || {}; | ||
|
||
var missingExternalDependecy = function(dep, dependencies) { | ||
return !_.contains(_.keys(dependencies), dep); | ||
}; | ||
|
||
return [ | ||
function(context, req, callback) { | ||
if (/^[a-z@][a-z\-\/0-9]+$/i.test(req)) { | ||
var dependencyName = req; | ||
if (/\//g.test(dependencyName)) { | ||
dependencyName = dependencyName.substring(0, dependencyName.indexOf('/')); | ||
} | ||
if (missingExternalDependecy(dependencyName, deps)) { | ||
return callback(new Error(format(strings.errors.cli.SERVERJS_DEPENDENCY_NOT_DECLARED, JSON.stringify(dependencyName)))); | ||
} | ||
} | ||
callback(); | ||
}, | ||
/^[a-z@][a-z\-\/0-9]+$/i | ||
]; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
/*jshint camelcase:false */ | ||
'use strict'; | ||
|
||
var webpack = require('webpack'); | ||
var path = require('path'); | ||
var wrapLoops = require('./wrapLoops'); | ||
var externalDependenciesHandlers = require('./externalDependenciesHandlers'); | ||
|
||
module.exports = function webpackConfigGenerator(params){ | ||
return { | ||
entry: params.dataPath, | ||
target: 'node', | ||
output: { | ||
path: '/build', | ||
filename: params.fileName, | ||
libraryTarget: 'commonjs2', | ||
}, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be be setting There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. good catch! |
||
externals: externalDependenciesHandlers(params.dependencies), | ||
module: { | ||
loaders: [ | ||
{ | ||
test: /\.json$/, | ||
exclude: /node_modules/, | ||
loader: 'json-loader' | ||
}, | ||
{ | ||
test: /\.js?$/, | ||
exclude: /node_modules/, | ||
loaders: [ | ||
'falafel-loader', | ||
'babel-loader?' + JSON.stringify({ | ||
cacheDirectory: true, | ||
'presets': [ | ||
[require.resolve('babel-preset-env'), { | ||
'targets': { | ||
'node': 4 | ||
} | ||
}] | ||
] | ||
}) | ||
], | ||
} | ||
] | ||
}, | ||
plugins: [ | ||
new webpack.optimize.OccurenceOrderPlugin(), | ||
new webpack.optimize.UglifyJsPlugin({ | ||
compressor: { | ||
warnings: false, | ||
screw_ie8: true | ||
}, | ||
sourceMap: false | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason for not having source-maps? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. At the moment we are focusing on having a production grade-output only. Not sure we wanted to have sourceMaps outputted now. What you guys recon? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't think we need sourcemaps on the server.js. Definitely will when we'll use this for client-side stuff, but that's not this PR's scope There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To be honest it's really useful when debugging production issues. The times that I've had to step through minified code and back reference that we the uncompiled stuff has been a lot. Maybe it shouldn't hold up this PR but it'd be a great addition IMO. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree with @chriscartlidge , I think that once we'll have the whole es6 server+client bundle workflow in place, we should definitely add those in. |
||
}), | ||
new webpack.DefinePlugin({ | ||
'process.env.NODE_ENV': JSON.stringify('production') | ||
}) | ||
], | ||
falafel: wrapLoops, | ||
resolveLoader: { | ||
root: path.resolve(__dirname, '../../../../../../node_modules') | ||
} | ||
}; | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,27 @@ | ||
'use strict'; | ||
|
||
|
||
var CONST_MAX_ITERATIONS = require('../../../../../resources/settings').maxLoopIterations; | ||
|
||
module.exports = function wrapLoops(node){ | ||
var loopKeywords = ['WhileStatement', 'ForStatement', 'DoWhileStatement']; | ||
|
||
if(loopKeywords.indexOf(node.type) > -1){ | ||
node.update( | ||
'var __ITER = ' + CONST_MAX_ITERATIONS + ';' | ||
+ node.source() | ||
); | ||
} | ||
|
||
if(!node.parent){ | ||
return; | ||
} | ||
|
||
if(loopKeywords.indexOf(node.parent.type) > -1 && node.type === 'BlockStatement'){ | ||
node.update('{ if(__ITER <=0){ throw new Error("Loop exceeded maximum ' | ||
+ 'allowed iterations"); } ' | ||
+ node.source().substr(1).slice(0, -1) | ||
+ ' __ITER--; }' | ||
); | ||
} | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,40 @@ | ||
/*jshint camelcase:false */ | ||
'use strict'; | ||
var webpackConfig = require('./config'); | ||
var console = require('console'); | ||
var MemoryFS = require('memory-fs'); | ||
var webpack = require('webpack'); | ||
|
||
var memoryFs = new MemoryFS(); | ||
|
||
module.exports = function bundle(params, callBack) { | ||
var config = webpackConfig(params); | ||
var compiler = webpack(config); | ||
compiler.outputFileSystem = memoryFs; | ||
|
||
compiler.run(function(error, stats){ | ||
var softError; | ||
var warning; | ||
|
||
// handleFatalError | ||
if (error) { | ||
return callBack(error); | ||
} | ||
|
||
var info = stats.toJson(); | ||
// handleSoftErrors | ||
if (stats.hasErrors()) { | ||
softError = info.errors.toString(); | ||
return callBack(softError); | ||
} | ||
// handleWarnings | ||
if (stats.hasWarnings()) { | ||
warning = info.warnings.toString(); | ||
} | ||
|
||
console.log(stats.toString(params.webpack.stats)); | ||
|
||
var serverContentBundled = memoryFs.readFileSync('/build/server.js', 'UTF8'); | ||
callBack(warning, serverContentBundled); | ||
}); | ||
}; |
This file was deleted.
This file was deleted.
This file was deleted.
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.
Worth upgrading to 2.0 since the new hot sauce has been released... Maybe too soon?
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.
If it's out of beta I 👍 on this, unless it requires a significant amount of work so that we want to keep it to the next PR
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.
Yes, and thats great news 🎉
Although, until we finally drop ourself support for node 0.10&co I will suggest we stick to v1. I'll prepare a v2 upgrade on a separate PR, what you think?
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.
ok @nickbalestra